From da8bb27703e301106e9f72dae7b05c13f7f621aa Mon Sep 17 00:00:00 2001 From: Tom Yu Date: Mon, 28 Jun 2004 22:47:11 +0000 Subject: [PATCH] Lots of signedness and argument-casting fixes. Some arithmetic paranoia for seasoning. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16526 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/rpc/ChangeLog | 44 +++++++++++++++++++++++++++++++++++++ src/lib/rpc/auth_gss.c | 12 +++++----- src/lib/rpc/auth_gssapi.c | 5 +++-- src/lib/rpc/authgss_prot.c | 15 +++++++++---- src/lib/rpc/clnt_udp.c | 8 +++---- src/lib/rpc/pmap_prot.c | 8 +++---- src/lib/rpc/rpc_prot.c | 14 ++++++------ src/lib/rpc/svc_auth_unix.c | 2 +- src/lib/rpc/xdr.c | 9 +++++--- src/lib/rpc/xdr.h | 36 +++++++++++++++++------------- src/lib/rpc/xdr_mem.c | 2 +- src/lib/rpc/xdr_rec.c | 16 ++++++++------ 12 files changed, 118 insertions(+), 53 deletions(-) diff --git a/src/lib/rpc/ChangeLog b/src/lib/rpc/ChangeLog index 141e9739f..a2f037137 100644 --- a/src/lib/rpc/ChangeLog +++ b/src/lib/rpc/ChangeLog @@ -1,3 +1,47 @@ +2004-06-28 Tom Yu + + * auth_gss.c (g_OID_equal): Fix signedness. + (rpc_gss_data): Fix width of WIN. + (authgss_validate): Fix width of NUM and QOP_STATE. + (authgss_refresh): Fix width of SEQ and QOP_STATE. + + * auth_gssapi.c (auth_gssapi_create): Save clnt->cl_auth early + enough to avoid unref use. + + * authgss_prot.c (xdr_rpc_gss_buf): Cast (void **) to (char **) + in call to xdr_bytes. + (xdr_rpc_gss_wrap_data): Fix signedness. + (xdr_rpc_gss_unwrap_data): Fix signedness. Fix width of SEQ_NUM. + + * clnt_udp.c (clntudp_bufcreate, clntudp_call, clntudp_call): + Fix up some argument casting for socket calls. + + * pmap_prot.c (xdr_pmap): Use appropriate xdr macros for the + typedefs instead of xdr_u_int32. + + * rpc_prot.c (xdr_accepted_reply, xdr_rejected_reply) + (xdr_callhdr): Use appropriate xdr macros for the typedefs instead + of xdr_u_int32. + + * svc_auth_unix.c (gssrpc__svcauth_unix): Fix signedness on call + to XDR_INLINE. + + * xdr.c (xdr_int, xdr_long, xdr_short): Improve value checks. + + * xdr.h: Make the IXDR macros more paranoid about signedness. Add + macros for xdr_rpcprog, xdr_rpcvers, xdr_rpcprot, xdr_rpcproc, + xdr_rpcport. + + * xdr_mem.c (xdrmem_getlong): Cast return value of ntohl to + int32_t prior to casting it to long. + + * xdr_rec.c (xdrrec_getlong): Cast return value of ntohl to + int32_t prior to casting it to long. + (xdrrec_putlong): Make arithmetic more paranoid. + (xdrrec_inline): Signedness fixes. Arithmetic paranoia. + (set_input_fragment): Don't cast return value of ntohl which is + being assigned to uint32_t. + 2004-06-25 Tom Yu * types.hin: Delete rpc_int32, rpc_u_int32 aliases. diff --git a/src/lib/rpc/auth_gss.c b/src/lib/rpc/auth_gss.c index 846cf36fc..982973a7c 100644 --- a/src/lib/rpc/auth_gss.c +++ b/src/lib/rpc/auth_gss.c @@ -140,7 +140,7 @@ print_rpc_gss_sec(struct rpc_gss_sec *ptr) #define g_OID_equal(o1,o2) \ (((o1)->length == (o2)->length) && \ ((o1)->elements != 0) && ((o2)->elements != 0) && \ - (memcmp((o1)->elements,(o2)->elements,(int) (o1)->length) == 0)) + (memcmp((o1)->elements, (o2)->elements, (o1)->length) == 0)) extern const gss_OID_desc * const gss_mech_krb5; #ifdef SPKM @@ -166,7 +166,7 @@ struct rpc_gss_data { struct rpc_gss_sec sec; /* security tuple */ gss_ctx_id_t ctx; /* context id */ struct rpc_gss_cred gc; /* client credentials */ - u_int win; /* sequence window */ + uint32_t win; /* sequence window */ }; #define AUTH_PRIVATE(auth) ((struct rpc_gss_data *)auth->ah_private) @@ -297,7 +297,7 @@ authgss_marshal(AUTH *auth, XDR *xdrs) /* Checksum serialized RPC header, up to and including credential. */ rpcbuf.length = XDR_GETPOS(xdrs); XDR_SETPOS(xdrs, 0); - rpcbuf.value = XDR_INLINE(xdrs, rpcbuf.length); + rpcbuf.value = XDR_INLINE(xdrs, (int)rpcbuf.length); maj_stat = gss_get_mic(&min_stat, gd->ctx, gd->sec.qop, &rpcbuf, &checksum); @@ -324,7 +324,8 @@ static bool_t authgss_validate(AUTH *auth, struct opaque_auth *verf) { struct rpc_gss_data *gd; - u_int num, qop_state; + uint32_t num; + gss_qop_t qop_state; gss_buffer_desc signbuf, checksum; OM_uint32 maj_stat, min_stat; @@ -466,7 +467,8 @@ authgss_refresh(AUTH *auth, struct rpc_msg *msg) if (maj_stat == GSS_S_COMPLETE) { gss_buffer_desc bufin; gss_buffer_desc bufout; - u_int seq, qop_state = 0; + uint32_t seq; + gss_qop_t qop_state = 0; seq = htonl(gr.gr_win); bufin.value = (u_char *)&seq; diff --git a/src/lib/rpc/auth_gssapi.c b/src/lib/rpc/auth_gssapi.c index 0efc793fb..d38097ba4 100644 --- a/src/lib/rpc/auth_gssapi.c +++ b/src/lib/rpc/auth_gssapi.c @@ -168,6 +168,9 @@ AUTH *auth_gssapi_create(clnt, gssstat, minor_stat, auth = NULL; pdata = NULL; + /* don't assume the caller will want to change clnt->cl_auth */ + save_auth = clnt->cl_auth; + auth = (AUTH *) malloc(sizeof(*auth)); pdata = (struct auth_gssapi_data *) malloc(sizeof(*pdata)); if (auth == NULL || pdata == NULL) { @@ -194,8 +197,6 @@ AUTH *auth_gssapi_create(clnt, gssstat, minor_stat, AUTH_PRIVATE(auth)->def_cred = (claimant_cred_handle == GSS_C_NO_CREDENTIAL); - /* don't assume the caller will want to change clnt->cl_auth */ - save_auth = clnt->cl_auth; clnt->cl_auth = auth; /* start by trying latest version */ diff --git a/src/lib/rpc/authgss_prot.c b/src/lib/rpc/authgss_prot.c index 322498519..0e8029abd 100644 --- a/src/lib/rpc/authgss_prot.c +++ b/src/lib/rpc/authgss_prot.c @@ -58,7 +58,7 @@ xdr_rpc_gss_buf(XDR *xdrs, gss_buffer_t buf, u_int maxsize) else tmplen = buf->length; } - xdr_stat = xdr_bytes(xdrs, &buf->value, &tmplen, maxsize); + xdr_stat = xdr_bytes(xdrs, (char **)&buf->value, &tmplen, maxsize); if (xdr_stat && xdrs->x_op == XDR_DECODE) buf->length = tmplen; @@ -131,23 +131,28 @@ xdr_rpc_gss_wrap_data(XDR *xdrs, xdrproc_t xdr_func, caddr_t xdr_ptr, { gss_buffer_desc databuf, wrapbuf; OM_uint32 maj_stat, min_stat; - int start, end, conf_state; + u_int start, end; + int conf_state; bool_t xdr_stat; u_int tmplen; /* Skip databody length. */ start = XDR_GETPOS(xdrs); + if (start > UINT_MAX - 4) + return (FALSE); XDR_SETPOS(xdrs, start + 4); /* Marshal rpc_gss_data_t (sequence number + arguments). */ if (!xdr_u_int32(xdrs, &seq) || !(*xdr_func)(xdrs, xdr_ptr)) return (FALSE); end = XDR_GETPOS(xdrs); + if (end < start + 4) + return (FALSE); /* Set databuf to marshalled rpc_gss_data_t. */ databuf.length = end - start - 4; XDR_SETPOS(xdrs, start + 4); - databuf.value = XDR_INLINE(xdrs, databuf.length); + databuf.value = XDR_INLINE(xdrs, (int)databuf.length); xdr_stat = FALSE; @@ -198,7 +203,9 @@ xdr_rpc_gss_unwrap_data(XDR *xdrs, xdrproc_t xdr_func, caddr_t xdr_ptr, XDR tmpxdrs; gss_buffer_desc databuf, wrapbuf; OM_uint32 maj_stat, min_stat; - u_int seq_num, conf_state, qop_state; + uint32_t seq_num; + int conf_state; + gss_qop_t qop_state; bool_t xdr_stat; if (xdr_func == xdr_void || xdr_ptr == NULL) diff --git a/src/lib/rpc/clnt_udp.c b/src/lib/rpc/clnt_udp.c index d245cb9be..33d3b0ec1 100644 --- a/src/lib/rpc/clnt_udp.c +++ b/src/lib/rpc/clnt_udp.c @@ -188,10 +188,10 @@ clntudp_bufcreate(raddr, program, version, wait, sockp, sendsz, recvsz) } else { cu->cu_closeit = FALSE; } - if (connect(*sockp, raddr, sizeof(*raddr)) < 0) + if (connect(*sockp, (struct sockaddr *)raddr, sizeof(*raddr)) < 0) goto fooy; cu->cu_llen = sizeof(cu->cu_laddr); - if (getsockname(*sockp, &cu->cu_laddr, &cu->cu_llen) < 0) + if (getsockname(*sockp, (struct sockaddr *)&cu->cu_laddr, &cu->cu_llen) < 0) goto fooy; cu->cu_sock = *sockp; @@ -272,7 +272,7 @@ call_again: outlen = (int)XDR_GETPOS(xdrs); send_again: - if (send(cu->cu_sock, cu->cu_outbuf, outlen, 0) != outlen) { + if (send(cu->cu_sock, cu->cu_outbuf, (u_int)outlen, 0) != outlen) { cu->cu_error.re_errno = errno; return (cu->cu_error.re_status = RPC_CANTSEND); } @@ -329,7 +329,7 @@ send_again: do { fromlen = sizeof(struct sockaddr); inlen = recvfrom(cu->cu_sock, cu->cu_inbuf, - (int) cu->cu_recvsz, 0, + cu->cu_recvsz, 0, (struct sockaddr *)&from, &fromlen); } while (inlen < 0 && errno == EINTR); if (inlen < 0) { diff --git a/src/lib/rpc/pmap_prot.c b/src/lib/rpc/pmap_prot.c index c1f25ce89..0dc6a5c46 100644 --- a/src/lib/rpc/pmap_prot.c +++ b/src/lib/rpc/pmap_prot.c @@ -49,9 +49,9 @@ xdr_pmap(xdrs, regs) struct pmap *regs; { - if (xdr_u_int32(xdrs, ®s->pm_prog) && - xdr_u_int32(xdrs, ®s->pm_vers) && - xdr_u_int32(xdrs, ®s->pm_prot)) - return (xdr_u_int32(xdrs, ®s->pm_port)); + if (xdr_rpcprog(xdrs, ®s->pm_prog) && + xdr_rpcvers(xdrs, ®s->pm_vers) && + xdr_rpcprot(xdrs, ®s->pm_prot)) + return (xdr_rpcport(xdrs, ®s->pm_port)); return (FALSE); } diff --git a/src/lib/rpc/rpc_prot.c b/src/lib/rpc/rpc_prot.c index 26e0088ed..4f282fb84 100644 --- a/src/lib/rpc/rpc_prot.c +++ b/src/lib/rpc/rpc_prot.c @@ -99,9 +99,9 @@ xdr_accepted_reply(xdrs, ar) return ((*(ar->ar_results.proc))(xdrs, ar->ar_results.where)); case PROG_MISMATCH: - if (! xdr_u_int32(xdrs, &(ar->ar_vers.low))) + if (! xdr_rpcvers(xdrs, &(ar->ar_vers.low))) return (FALSE); - return (xdr_u_int32(xdrs, &(ar->ar_vers.high))); + return (xdr_rpcvers(xdrs, &(ar->ar_vers.high))); case GARBAGE_ARGS: case SYSTEM_ERR: @@ -127,9 +127,9 @@ xdr_rejected_reply(xdrs, rr) switch (rr->rj_stat) { case RPC_MISMATCH: - if (! xdr_u_int32(xdrs, &(rr->rj_vers.low))) + if (! xdr_rpcvers(xdrs, &(rr->rj_vers.low))) return (FALSE); - return (xdr_u_int32(xdrs, &(rr->rj_vers.high))); + return (xdr_rpcvers(xdrs, &(rr->rj_vers.high))); case AUTH_ERROR: return (xdr_enum(xdrs, (enum_t *)&(rr->rj_why))); @@ -177,9 +177,9 @@ xdr_callhdr(xdrs, cmsg) (xdrs->x_op == XDR_ENCODE) && xdr_u_int32(xdrs, &(cmsg->rm_xid)) && xdr_enum(xdrs, (enum_t *)&(cmsg->rm_direction)) && - xdr_u_int32(xdrs, &(cmsg->rm_call.cb_rpcvers)) && - xdr_u_int32(xdrs, &(cmsg->rm_call.cb_prog)) ) - return (xdr_u_int32(xdrs, &(cmsg->rm_call.cb_vers))); + xdr_rpcvers(xdrs, &(cmsg->rm_call.cb_rpcvers)) && + xdr_rpcprog(xdrs, &(cmsg->rm_call.cb_prog)) ) + return (xdr_rpcvers(xdrs, &(cmsg->rm_call.cb_vers))); return (FALSE); } diff --git a/src/lib/rpc/svc_auth_unix.c b/src/lib/rpc/svc_auth_unix.c index cb4a30c31..eb8182de0 100644 --- a/src/lib/rpc/svc_auth_unix.c +++ b/src/lib/rpc/svc_auth_unix.c @@ -76,7 +76,7 @@ gssrpc__svcauth_unix(rqst, msg, dispatch) aup->aup_gids = area->area_gids; auth_len = (u_int)msg->rm_call.cb_cred.oa_length; xdrmem_create(&xdrs, msg->rm_call.cb_cred.oa_base, auth_len,XDR_DECODE); - buf = XDR_INLINE(&xdrs, auth_len); + buf = XDR_INLINE(&xdrs, (int)auth_len); if (buf != NULL) { aup->aup_time = IXDR_GET_LONG(buf); str_len = IXDR_GET_U_LONG(buf); diff --git a/src/lib/rpc/xdr.c b/src/lib/rpc/xdr.c index 3bec45197..264035c54 100644 --- a/src/lib/rpc/xdr.c +++ b/src/lib/rpc/xdr.c @@ -99,7 +99,7 @@ xdr_int(xdrs, ip) switch (xdrs->x_op) { case XDR_ENCODE: - if (*ip > 0x7fffffffL) + if (*ip > 0x7fffffffL || *ip < -0x7fffffffL - 1L) return (FALSE); l = (long) *ip; @@ -109,7 +109,7 @@ xdr_int(xdrs, ip) if (!XDR_GETLONG(xdrs, &l)) return (FALSE); - if ((u_long)l > UINT_MAX || l < INT_MIN) + if (l > INT_MAX || l < INT_MIN) return (FALSE); *ip = (int) l; @@ -168,7 +168,7 @@ xdr_long(xdrs, lp) switch (xdrs->x_op) { case XDR_ENCODE: - if (*lp > 0x7fffffffL) + if (*lp > 0x7fffffffL || *lp < -0x7fffffffL - 1L) return (FALSE); return (XDR_PUTLONG(xdrs, lp)); @@ -227,6 +227,9 @@ xdr_short(xdrs, sp) if (!XDR_GETLONG(xdrs, &l)) { return (FALSE); } + if (l > SHRT_MAX || l < SHRT_MIN) + return (FALSE); + *sp = (short) l; return (TRUE); diff --git a/src/lib/rpc/xdr.h b/src/lib/rpc/xdr.h index 6b982dc8e..943e39e2a 100644 --- a/src/lib/rpc/xdr.h +++ b/src/lib/rpc/xdr.h @@ -223,25 +223,25 @@ struct xdr_discrim { * N.B. and frozen for all time: each data type here uses 4 bytes * of external representation. */ -#define IXDR_GET_INT32(buf) ((int32_t)ntohl((uint32_t)*(buf)++)) -#define IXDR_PUT_INT32(buf, v) (*(buf)++ = (int32_t)htonl((uint32_t)(v))) -#define IXDR_GET_U_INT32(buf) ((uint32_t)IXDR_GET_INT32(buf)) -#define IXDR_PUT_U_INT32(buf, v) IXDR_PUT_INT32(buf, (int32_t)(v)) +#define IXDR_GET_INT32(buf) ((int32_t)IXDR_GET_U_INT32(buf)) +#define IXDR_PUT_INT32(buf, v) IXDR_PUT_U_INT32((buf),((uint32_t)(v))) +#define IXDR_GET_U_INT32(buf) (ntohl((uint32_t)*(buf)++)) +#define IXDR_PUT_U_INT32(buf, v) (*(buf)++ = (int32_t)htonl((v))) -#define IXDR_GET_LONG(buf) ((long)ntohl((uint32_t)*(buf)++)) -#define IXDR_PUT_LONG(buf, v) (*(buf)++ = (int32_t)htonl((uint32_t)(v))) +#define IXDR_GET_LONG(buf) ((long)IXDR_GET_INT32(buf)) +#define IXDR_PUT_LONG(buf, v) IXDR_PUT_U_INT32((buf),((uint32_t)(v))) #define IXDR_GET_BOOL(buf) ((bool_t)IXDR_GET_LONG(buf)) -#define IXDR_GET_ENUM(buf, t) ((t)IXDR_GET_LONG(buf)) -#define IXDR_GET_U_LONG(buf) ((u_long)IXDR_GET_LONG(buf)) -#define IXDR_GET_SHORT(buf) ((short)IXDR_GET_LONG(buf)) -#define IXDR_GET_U_SHORT(buf) ((u_short)IXDR_GET_LONG(buf)) +#define IXDR_GET_ENUM(buf, t) ((t)IXDR_GET_INT32(buf)) +#define IXDR_GET_U_LONG(buf) ((u_long)IXDR_GET_U_INT32(buf)) +#define IXDR_GET_SHORT(buf) ((short)IXDR_GET_INT32(buf)) +#define IXDR_GET_U_SHORT(buf) ((u_short)IXDR_GET_U_INT32(buf)) -#define IXDR_PUT_BOOL(buf, v) IXDR_PUT_LONG((buf), ((long)(v))) -#define IXDR_PUT_ENUM(buf, v) IXDR_PUT_LONG((buf), ((long)(v))) -#define IXDR_PUT_U_LONG(buf, v) IXDR_PUT_LONG((buf), ((long)(v))) -#define IXDR_PUT_SHORT(buf, v) IXDR_PUT_LONG((buf), ((long)(v))) -#define IXDR_PUT_U_SHORT(buf, v) IXDR_PUT_LONG((buf), ((long)(v))) +#define IXDR_PUT_BOOL(buf, v) IXDR_PUT_INT32((buf),((int32_t)(v))) +#define IXDR_PUT_ENUM(buf, v) IXDR_PUT_INT32((buf),((int32_t)(v))) +#define IXDR_PUT_U_LONG(buf, v) IXDR_PUT_U_INT32((buf),((uint32_t)(v))) +#define IXDR_PUT_SHORT(buf, v) IXDR_PUT_INT32((buf),((int32_t)(v))) +#define IXDR_PUT_U_SHORT(buf, v) IXDR_PUT_U_INT32((buf),((uint32_t)(v))) /* * These are the "generic" xdr routines. @@ -271,6 +271,12 @@ extern bool_t xdr_reference(XDR *, caddr_t *, u_int, xdrproc_t); extern bool_t xdr_pointer(XDR *, char **, u_int, xdrproc_t); extern bool_t xdr_wrapstring(XDR *, char **); +#define xdr_rpcprog xdr_u_int32 +#define xdr_rpcvers xdr_u_int32 +#define xdr_rpcprot xdr_u_int32 +#define xdr_rpcproc xdr_u_int32 +#define xdr_rpcport xdr_u_int32 + /* * Common opaque bytes objects used by many rpc protocols; * declared here due to commonality. diff --git a/src/lib/rpc/xdr_mem.c b/src/lib/rpc/xdr_mem.c index 2e4769977..39be29642 100644 --- a/src/lib/rpc/xdr_mem.c +++ b/src/lib/rpc/xdr_mem.c @@ -104,7 +104,7 @@ xdrmem_getlong(xdrs, lp) return (FALSE); else xdrs->x_handy -= BYTES_PER_XDR_UNIT; - *lp = (long)ntohl(*((uint32_t *)(xdrs->x_private))); + *lp = (long)(int32_t)ntohl(*((uint32_t *)(xdrs->x_private))); xdrs->x_private = (char *)xdrs->x_private + BYTES_PER_XDR_UNIT; return (TRUE); } diff --git a/src/lib/rpc/xdr_rec.c b/src/lib/rpc/xdr_rec.c index ae79627e2..eefd9db1f 100644 --- a/src/lib/rpc/xdr_rec.c +++ b/src/lib/rpc/xdr_rec.c @@ -213,7 +213,7 @@ xdrrec_getlong(xdrs, lp) if (! xdrrec_getbytes(xdrs, (caddr_t)&mylong, BYTES_PER_XDR_UNIT)) return (FALSE); - *lp = (long)ntohl(mylong); + *lp = (long)(int32_t)ntohl(mylong); } return (TRUE); } @@ -226,18 +226,17 @@ xdrrec_putlong(xdrs, lp) register RECSTREAM *rstrm = (RECSTREAM *)(xdrs->x_private); register int32_t *dest_lp = ((int32_t *)(void *)(rstrm->out_finger)); - if ((rstrm->out_finger += BYTES_PER_XDR_UNIT) > rstrm->out_boundry) { + if (rstrm->out_boundry - rstrm->out_finger < BYTES_PER_XDR_UNIT) { /* * this case should almost never happen so the code is * inefficient */ - rstrm->out_finger -= BYTES_PER_XDR_UNIT; rstrm->frag_sent = TRUE; if (! flush_out(rstrm, FALSE)) return (FALSE); dest_lp = ((int32_t *)(void *)(rstrm->out_finger)); - rstrm->out_finger += BYTES_PER_XDR_UNIT; } + rstrm->out_finger += BYTES_PER_XDR_UNIT; *dest_lp = (int32_t)htonl((uint32_t)(*lp)); return (TRUE); } @@ -369,10 +368,13 @@ xdrrec_inline(xdrs, len) register RECSTREAM *rstrm = (RECSTREAM *)xdrs->x_private; rpc_inline_t * buf = NULL; + if (len < 0) + return (FALSE); + switch (xdrs->x_op) { case XDR_ENCODE: - if ((rstrm->out_finger + len) <= rstrm->out_boundry) { + if (len <= (rstrm->out_boundry - rstrm->out_finger)) { buf = (rpc_inline_t *)(void *) rstrm->out_finger; rstrm->out_finger += len; } @@ -380,7 +382,7 @@ xdrrec_inline(xdrs, len) case XDR_DECODE: if ((len <= rstrm->fbtbc) && - ((rstrm->in_finger + len) <= rstrm->in_boundry)) { + (len <= (rstrm->in_boundry - rstrm->in_finger))) { buf = (rpc_inline_t *)(void *) rstrm->in_finger; rstrm->fbtbc -= len; rstrm->in_finger += len; @@ -557,7 +559,7 @@ set_input_fragment(rstrm) if (! get_input_bytes(rstrm, (caddr_t)&header, sizeof(header))) return (FALSE); - header = (int)ntohl(header); + header = ntohl(header); rstrm->last_frag = ((header & LAST_FRAG) == 0) ? FALSE : TRUE; rstrm->fbtbc = header & (~LAST_FRAG); return (TRUE); -- 2.26.2