On 2015/10/14 11:11, Kevin Reay wrote:
> Thanks for the review and feedback.
> Updated patch with removed whitespace changes included.
This is fine with me. Any OKs to commit it?
> Index: print-decnet.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/print-decnet.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 print-decnet.c
> --- print-decnet.c 21 Aug 2015 02:07:32 -0000 1.14
> +++ print-decnet.c 14 Oct 2015 22:49:03 -0000
> @@ -44,13 +44,13 @@ struct rtentry;
> #include "addrtoname.h"
>
> /* Forwards */
> -static void print_decnet_ctlmsg(const union routehdr *, u_int);
> +static int print_decnet_ctlmsg(const union routehdr *, u_int, u_int);
> static void print_t_info(int);
> -static void print_l1_routes(const char *, u_int);
> -static void print_l2_routes(const char *, u_int);
> +static int print_l1_routes(const char *, u_int);
> +static int print_l2_routes(const char *, u_int);
> static void print_i_info(int);
> -static void print_elist(const char *, u_int);
> -static void print_nsp(const u_char *, u_int);
> +static int print_elist(const char *, u_int);
> +static int print_nsp(const u_char *, u_int);
> static void print_reason(int);
> #ifdef PRINT_NSPDATA
> static void pdata(u_char *, int);
> @@ -76,12 +76,23 @@ decnet_print(register const u_char *ap,
> return;
> }
>
> + TCHECK2(*ap, sizeof(short));
> pktlen = EXTRACT_LE_16BITS(ap);
> + if (pktlen < sizeof(struct shorthdr)) {
> + (void)printf("[|decnet]");
> + return;
> + }
> + if (pktlen > length) {
> + (void)printf("[|decnet]");
> + return;
> + }
> + length = pktlen;
>
> rhlen = min(length, caplen);
> rhlen = min(rhlen, sizeof(*rhp));
> memcpy((char *)rhp, (char *)&(ap[sizeof(short)]), rhlen);
>
> + TCHECK(rhp->rh_short.sh_flags);
> mflags = EXTRACT_LE_8BITS(rhp->rh_short.sh_flags);
>
> if (mflags & RMF_PAD) {
> @@ -89,6 +100,11 @@ decnet_print(register const u_char *ap,
> u_int padlen = mflags & RMF_PADMASK;
> if (vflag)
> (void) printf("[pad:%d] ", padlen);
> + if (length < padlen + 2) {
> + (void)printf("[|decnet]");
> + return;
> + }
> + TCHECK2(ap[sizeof(short)], padlen);
> ap += padlen;
> length -= padlen;
> caplen -= padlen;
> @@ -100,38 +116,43 @@ decnet_print(register const u_char *ap,
>
> if (mflags & RMF_FVER) {
> (void) printf("future-version-decnet");
> - default_print(ap, length);
> + default_print(ap, min(length, caplen));
> return;
> }
>
> /* is it a control message? */
> if (mflags & RMF_CTLMSG) {
> - print_decnet_ctlmsg(rhp, min(length, caplen));
> + if(!print_decnet_ctlmsg(rhp, length, caplen))
> + goto trunc;
> return;
> }
>
> switch (mflags & RMF_MASK) {
> case RMF_LONG:
> + if (length < sizeof(struct longhdr)) {
> + (void)printf("[|decnet]");
> + return;
> + }
> + TCHECK(rhp->rh_long);
> dst =
> EXTRACT_LE_16BITS(rhp->rh_long.lg_dst.dne_remote.dne_nodeaddr);
> src =
> EXTRACT_LE_16BITS(rhp->rh_long.lg_src.dne_remote.dne_nodeaddr);
> hops = EXTRACT_LE_8BITS(rhp->rh_long.lg_visits);
> nspp = &(ap[sizeof(short) + sizeof(struct longhdr)]);
> - nsplen = min((length - sizeof(struct longhdr)),
> - (caplen - sizeof(struct longhdr)));
> + nsplen = length - sizeof(struct longhdr);
> break;
> case RMF_SHORT:
> + TCHECK(rhp->rh_short);
> dst = EXTRACT_LE_16BITS(rhp->rh_short.sh_dst);
> src = EXTRACT_LE_16BITS(rhp->rh_short.sh_src);
> hops = (EXTRACT_LE_8BITS(rhp->rh_short.sh_visits) & VIS_MASK)+1;
> nspp = &(ap[sizeof(short) + sizeof(struct shorthdr)]);
> - nsplen = min((length - sizeof(struct shorthdr)),
> - (caplen - sizeof(struct shorthdr)));
> + nsplen = length - sizeof(struct shorthdr);
> break;
> default:
> (void) printf("unknown message flags under mask");
> - default_print((u_char *)ap, length);
> + default_print((u_char *)ap, min(length, caplen));
> return;
> }
>
> @@ -147,11 +168,18 @@ decnet_print(register const u_char *ap,
> (void)printf("%d hops ", hops);
> }
>
> - print_nsp(nspp, nsplen);
> + if (!print_nsp(nspp, nsplen))
> + goto trunc;
> + return;
> +
> +trunc:
> + (void)printf("[|decnet]");
> + return;
> }
>
> -static void
> -print_decnet_ctlmsg(register const union routehdr *rhp, u_int length)
> +static int
> +print_decnet_ctlmsg(register const union routehdr *rhp, u_int length,
> + u_int caplen)
> {
> int mflags = EXTRACT_LE_8BITS(rhp->rh_short.sh_flags);
> register union controlmsg *cmp = (union controlmsg *)rhp;
> @@ -159,10 +187,14 @@ print_decnet_ctlmsg(register const union
> etheraddr srcea, rtea;
> int priority;
> char *rhpx = (char *)rhp;
> + int ret;
>
> switch (mflags & RMF_CTLMASK) {
> case RMF_INIT:
> (void)printf("init ");
> + if (length < sizeof(struct initmsg))
> + goto trunc;
> + TCHECK(cmp->cm_init);
> src = EXTRACT_LE_16BITS(cmp->cm_init.in_src);
> info = EXTRACT_LE_8BITS(cmp->cm_init.in_info);
> blksize = EXTRACT_LE_16BITS(cmp->cm_init.in_blksize);
> @@ -175,35 +207,54 @@ print_decnet_ctlmsg(register const union
> "src %sblksize %d vers %d eco %d ueco %d hello %d",
> dnaddr_string(src), blksize, vers, eco, ueco,
> hello);
> +
> + ret = 1;
> break;
> case RMF_VER:
> (void)printf("verification ");
> + if (length < sizeof(struct verifmsg))
> + goto trunc;
> + TCHECK(cmp->cm_ver);
> src = EXTRACT_LE_16BITS(cmp->cm_ver.ve_src);
> other = EXTRACT_LE_8BITS(cmp->cm_ver.ve_fcnval);
> (void)printf("src %s fcnval %o", dnaddr_string(src), other);
> + ret = 1;
> break;
> case RMF_TEST:
> (void)printf("test ");
> + if (length < sizeof(struct testmsg))
> + goto trunc;
> + TCHECK(cmp->cm_test);
> src = EXTRACT_LE_16BITS(cmp->cm_test.te_src);
> other = EXTRACT_LE_8BITS(cmp->cm_test.te_data);
> (void)printf("src %s data %o", dnaddr_string(src), other);
> + ret = 1;
> break;
> case RMF_L1ROUT:
> (void)printf("lev-1-routing ");
> + if (length < sizeof(struct l1rout))
> + goto trunc;
> + TCHECK(cmp->cm_l1rou);
> src = EXTRACT_LE_16BITS(cmp->cm_l1rou.r1_src);
> (void)printf("src %s ", dnaddr_string(src));
> - print_l1_routes(&(rhpx[sizeof(struct l1rout)]),
> + ret = print_l1_routes(&(rhpx[sizeof(struct l1rout)]),
> length - sizeof(struct l1rout));
> break;
> case RMF_L2ROUT:
> (void)printf("lev-2-routing ");
> + if (length < sizeof(struct l2rout))
> + goto trunc;
> + TCHECK(cmp->cm_l2rout);
> src = EXTRACT_LE_16BITS(cmp->cm_l2rout.r2_src);
> (void)printf("src %s ", dnaddr_string(src));
> - print_l2_routes(&(rhpx[sizeof(struct l2rout)]),
> + ret = print_l2_routes(&(rhpx[sizeof(struct l2rout)]),
> length - sizeof(struct l2rout));
> break;
> case RMF_RHELLO:
> (void)printf("router-hello ");
> + if (length < sizeof(struct rhellomsg))
> + goto trunc;
> + TCHECK(cmp->cm_rhello);
> vers = EXTRACT_LE_8BITS(cmp->cm_rhello.rh_vers);
> eco = EXTRACT_LE_8BITS(cmp->cm_rhello.rh_eco);
> ueco = EXTRACT_LE_8BITS(cmp->cm_rhello.rh_ueco);
> @@ -219,11 +270,14 @@ print_decnet_ctlmsg(register const union
> "vers %d eco %d ueco %d src %s blksize %d pri %d hello %d",
> vers, eco, ueco, dnaddr_string(src),
> blksize, priority, hello);
> - print_elist(&(rhpx[sizeof(struct rhellomsg)]),
> + ret = print_elist(&(rhpx[sizeof(struct rhellomsg)]),
> length - sizeof(struct rhellomsg));
> break;
> case RMF_EHELLO:
> (void)printf("endnode-hello ");
> + if (length < sizeof(struct ehellomsg))
> + goto trunc;
> + TCHECK(cmp->cm_ehello);
> vers = EXTRACT_LE_8BITS(cmp->cm_ehello.eh_vers);
> eco = EXTRACT_LE_8BITS(cmp->cm_ehello.eh_eco);
> ueco = EXTRACT_LE_8BITS(cmp->cm_ehello.eh_ueco);
> @@ -243,13 +297,19 @@ print_decnet_ctlmsg(register const union
> "vers %d eco %d ueco %d src %s blksize %d rtr %s hello %d data %o",
> vers, eco, ueco, dnaddr_string(src),
> blksize, dnaddr_string(dst), hello, other);
> + ret = 1;
> break;
>
> default:
> (void)printf("unknown control message");
> - default_print((u_char *)rhp, length);
> + default_print((u_char *)rhp, min(length, caplen));
> + ret = 1;
> break;
> }
> + return (ret);
> +
> +trunc:
> + return (0);
> }
>
> static void
> @@ -268,7 +328,7 @@ print_t_info(int info)
> (void)printf("blo ");
> }
>
> -static void
> +static int
> print_l1_routes(const char *rp, u_int len)
> {
> int count;
> @@ -277,9 +337,10 @@ print_l1_routes(const char *rp, u_int le
>
> /* The last short is a checksum */
> while (len > (3 * sizeof(short))) {
> + TCHECK2(*rp, 3 * sizeof(short));
> count = EXTRACT_LE_16BITS(rp);
> if (count > 1024)
> - return; /* seems to be bogus from here on */
> + return (1); /* seems to be bogus from here on */
> rp += sizeof(short);
> len -= sizeof(short);
> id = EXTRACT_LE_16BITS(rp);
> @@ -291,9 +352,13 @@ print_l1_routes(const char *rp, u_int le
> (void)printf("{ids %d-%d cost %d hops %d} ", id, id + count,
> RI_COST(info), RI_HOPS(info));
> }
> + return (1);
> +
> +trunc:
> + return (0);
> }
>
> -static void
> +static int
> print_l2_routes(const char *rp, u_int len)
> {
> int count;
> @@ -302,9 +367,10 @@ print_l2_routes(const char *rp, u_int le
>
> /* The last short is a checksum */
> while (len > (3 * sizeof(short))) {
> + TCHECK2(*rp, 3 * sizeof(short));
> count = EXTRACT_LE_16BITS(rp);
> if (count > 1024)
> - return; /* seems to be bogus from here on */
> + return (1); /* seems to be bogus from here on */
> rp += sizeof(short);
> len -= sizeof(short);
> area = EXTRACT_LE_16BITS(rp);
> @@ -316,6 +382,10 @@ print_l2_routes(const char *rp, u_int le
> (void)printf("{areas %d-%d cost %d hops %d} ", area, area + count,
> RI_COST(info), RI_HOPS(info));
> }
> + return (1);
> +
> +trunc:
> + return (0);
> }
>
> static void
> @@ -336,18 +406,22 @@ print_i_info(int info)
> (void)printf("blo ");
> }
>
> -static void
> +static int
> print_elist(const char *elp, u_int len)
> {
> /* Not enough examples available for me to debug this */
> + return (1);
> }
>
> -static void
> +static int
> print_nsp(const u_char *nspp, u_int nsplen)
> {
> const struct nsphdr *nsphp = (struct nsphdr *)nspp;
> int dst, src, flags;
>
> + if (nsplen < sizeof(struct nsphdr))
> + goto trunc;
> + TCHECK(*nsphp);
> flags = EXTRACT_LE_8BITS(nsphp->nh_flags);
> dst = EXTRACT_LE_16BITS(nsphp->nh_dst);
> src = EXTRACT_LE_16BITS(nsphp->nh_src);
> @@ -368,27 +442,39 @@ print_nsp(const u_char *nspp, u_int nspl
> #endif
> u_int data_off = sizeof(struct minseghdr);
>
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[0]);
> ack = EXTRACT_LE_16BITS(shp->sh_seq[0]);
> if (ack & SGQ_ACK) { /* acknum field */
> if ((ack & SGQ_NAK) == SGQ_NAK)
> (void)printf("nak %d ", ack & SGQ_MASK);
> else
> (void)printf("ack %d ", ack & SGQ_MASK);
> - ack = EXTRACT_LE_16BITS(shp->sh_seq[1]);
> - data_off += sizeof(short);
> + data_off += sizeof(short);
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[1]);
> + ack = EXTRACT_LE_16BITS(shp->sh_seq[1]);
> if (ack & SGQ_OACK) { /* ackoth field */
> if ((ack & SGQ_ONAK) == SGQ_ONAK)
> (void)printf("onak %d ", ack & SGQ_MASK);
> else
> (void)printf("oack %d ", ack & SGQ_MASK);
> - ack = EXTRACT_LE_16BITS(shp->sh_seq[2]);
> data_off += sizeof(short);
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[2]);
> + ack = EXTRACT_LE_16BITS(shp->sh_seq[2]);
> }
> }
> (void)printf("seg %d ", ack & SGQ_MASK);
> #ifdef PRINT_NSPDATA
> - dp = &(nspp[data_off]);
> - pdata(dp, 10);
> + if (nsplen > data_off) {
> + dp = &(nspp[data_off]);
> + TCHECK2(*dp, nsplen - data_off);
> + pdata(dp, nsplen - data_off);
> + }
> #endif
> }
> break;
> @@ -402,27 +488,39 @@ print_nsp(const u_char *nspp, u_int nspl
> #endif
> u_int data_off = sizeof(struct minseghdr);
>
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[0]);
> ack = EXTRACT_LE_16BITS(shp->sh_seq[0]);
> if (ack & SGQ_ACK) { /* acknum field */
> if ((ack & SGQ_NAK) == SGQ_NAK)
> (void)printf("nak %d ", ack & SGQ_MASK);
> else
> (void)printf("ack %d ", ack & SGQ_MASK);
> - ack = EXTRACT_LE_16BITS(shp->sh_seq[1]);
> - data_off += sizeof(short);
> + data_off += sizeof(short);
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[1]);
> + ack = EXTRACT_LE_16BITS(shp->sh_seq[1]);
> if (ack & SGQ_OACK) { /* ackdat field */
> if ((ack & SGQ_ONAK) == SGQ_ONAK)
> (void)printf("nakdat %d ", ack & SGQ_MASK);
> else
> (void)printf("ackdat %d ", ack & SGQ_MASK);
> - ack = EXTRACT_LE_16BITS(shp->sh_seq[2]);
> data_off += sizeof(short);
> + if (nsplen < data_off)
> + goto trunc;
> + TCHECK(shp->sh_seq[2]);
> + ack = EXTRACT_LE_16BITS(shp->sh_seq[2]);
> }
> }
> (void)printf("seg %d ", ack & SGQ_MASK);
> #ifdef PRINT_NSPDATA
> - dp = &(nspp[data_off]);
> - pdata(dp, 10);
> + if (nsplen > data_off) {
> + dp = &(nspp[data_off]);
> + TCHECK2(*dp, nsplen - data_off);
> + pdata(dp, nsplen - data_off);
> + }
> #endif
> }
> break;
> @@ -435,22 +533,28 @@ print_nsp(const u_char *nspp, u_int nspl
> int ack;
> int lsflags, fcval;
>
> + if (nsplen < sizeof(struct seghdr) + sizeof(struct lsmsg))
> + goto trunc;
> + TCHECK(shp->sh_seq[0]);
> ack = EXTRACT_LE_16BITS(shp->sh_seq[0]);
> if (ack & SGQ_ACK) { /* acknum field */
> if ((ack & SGQ_NAK) == SGQ_NAK)
> (void)printf("nak %d ", ack & SGQ_MASK);
> else
> (void)printf("ack %d ", ack & SGQ_MASK);
> + TCHECK(shp->sh_seq[1]);
> ack = EXTRACT_LE_16BITS(shp->sh_seq[1]);
> if (ack & SGQ_OACK) { /* ackdat field */
> if ((ack & SGQ_ONAK) == SGQ_ONAK)
> (void)printf("nakdat %d ", ack & SGQ_MASK);
> else
> (void)printf("ackdat %d ", ack & SGQ_MASK);
> + TCHECK(shp->sh_seq[2]);
> ack = EXTRACT_LE_16BITS(shp->sh_seq[2]);
> }
> }
> (void)printf("seg %d ", ack & SGQ_MASK);
> + TCHECK(*lsmp);
> lsflags = EXTRACT_LE_8BITS(lsmp->ls_lsflags);
> fcval = EXTRACT_LE_8BITS(lsmp->ls_fcval);
> switch (lsflags & LSI_MASK) {
> @@ -492,6 +596,9 @@ print_nsp(const u_char *nspp, u_int nspl
> struct ackmsg *amp = (struct ackmsg *)nspp;
> int ack;
>
> + if (nsplen < sizeof(struct ackmsg))
> + goto trunc;
> + TCHECK(*amp);
> ack = EXTRACT_LE_16BITS(amp->ak_acknum[0]);
> if (ack & SGQ_ACK) { /* acknum field */
> if ((ack & SGQ_NAK) == SGQ_NAK)
> @@ -514,13 +621,17 @@ print_nsp(const u_char *nspp, u_int nspl
> struct ackmsg *amp = (struct ackmsg *)nspp;
> int ack;
>
> + if (nsplen < sizeof(struct ackmsg))
> + goto trunc;
> + TCHECK(*amp);
> ack = EXTRACT_LE_16BITS(amp->ak_acknum[0]);
> if (ack & SGQ_ACK) { /* acknum field */
> if ((ack & SGQ_NAK) == SGQ_NAK)
> (void)printf("nak %d ", ack & SGQ_MASK);
> else
> (void)printf("ack %d ", ack & SGQ_MASK);
> - ack = EXTRACT_LE_16BITS(amp->ak_acknum[1]);
> + TCHECK(amp->ak_acknum[1]);
> + ack = EXTRACT_LE_16BITS(amp->ak_acknum[1]);
> if (ack & SGQ_OACK) { /* ackdat field */
> if ((ack & SGQ_ONAK) == SGQ_ONAK)
> (void)printf("nakdat %d ", ack & SGQ_MASK);
> @@ -554,6 +665,9 @@ print_nsp(const u_char *nspp, u_int nspl
> u_char *dp;
> #endif
>
> + if (nsplen < sizeof(struct cimsg))
> + goto trunc;
> + TCHECK(*cimp);
> services = EXTRACT_LE_8BITS(cimp->ci_services);
> info = EXTRACT_LE_8BITS(cimp->ci_info);
> segsize = EXTRACT_LE_16BITS(cimp->ci_segsize);
> @@ -587,8 +701,11 @@ print_nsp(const u_char *nspp, u_int nspl
> }
> (void)printf("segsize %d ", segsize);
> #ifdef PRINT_NSPDATA
> - dp = &(nspp[sizeof(struct cimsg)]);
> - pdata(dp, nsplen - sizeof(struct cimsg));
> + if (nsplen > sizeof(struct cimsg)) {
> + dp = &(nspp[sizeof(struct cimsg)]);
> + TCHECK2(*dp, nsplen - sizeof(struct cimsg));
> + pdata(dp, nsplen - sizeof(struct cimsg));
> + }
> #endif
> }
> break;
> @@ -602,6 +719,9 @@ print_nsp(const u_char *nspp, u_int nspl
> u_char *dp;
> #endif
>
> + if (nsplen < sizeof(struct ccmsg))
> + goto trunc;
> + TCHECK(*ccmp);
> services = EXTRACT_LE_8BITS(ccmp->cc_services);
> info = EXTRACT_LE_8BITS(ccmp->cc_info);
> segsize = EXTRACT_LE_16BITS(ccmp->cc_segsize);
> @@ -638,8 +758,10 @@ print_nsp(const u_char *nspp, u_int nspl
> if (optlen) {
> (void)printf("optlen %d ", optlen);
> #ifdef PRINT_NSPDATA
> - optlen = min(optlen, nsplen - sizeof(struct ccmsg));
> + if (optlen > nsplen - sizeof(struct ccmsg))
> + goto trunc;
> dp = &(nspp[sizeof(struct ccmsg)]);
> + TCHECK2(*dp, optlen);
> pdata(dp, optlen);
> #endif
> }
> @@ -655,6 +777,9 @@ print_nsp(const u_char *nspp, u_int nspl
> u_char *dp;
> #endif
>
> + if (nsplen < sizeof(struct dimsg))
> + goto trunc;
> + TCHECK(*dimp);
> reason = EXTRACT_LE_16BITS(dimp->di_reason);
> optlen = EXTRACT_LE_8BITS(dimp->di_optlen);
>
> @@ -662,8 +787,10 @@ print_nsp(const u_char *nspp, u_int nspl
> if (optlen) {
> (void)printf("optlen %d ", optlen);
> #ifdef PRINT_NSPDATA
> - optlen = min(optlen, nsplen - sizeof(struct dimsg));
> + if (optlen > nsplen - sizeof(struct dimsg))
> + goto trunc;
> dp = &(nspp[sizeof(struct dimsg)]);
> + TCHECK2(*dp, optlen);
> pdata(dp, optlen);
> #endif
> }
> @@ -675,6 +802,7 @@ print_nsp(const u_char *nspp, u_int nspl
> struct dcmsg *dcmp = (struct dcmsg *)nspp;
> int reason;
>
> + TCHECK(*dcmp);
> reason = EXTRACT_LE_16BITS(dcmp->dc_reason);
>
> print_reason(reason);
> @@ -689,6 +817,10 @@ print_nsp(const u_char *nspp, u_int nspl
> (void)printf("reserved-type? %x %d > %d", flags, src, dst);
> break;
> }
> + return (1);
> +
> +trunc:
> + return (0);
> }
>
> static struct tok reason2str[] = {
