]> The Tcpdump Group git mirrors - tcpdump/commitdiff
Add some attribute/TLV length checks.
authorGuy Harris <[email protected]>
Sat, 9 Jun 2012 02:07:20 +0000 (19:07 -0700)
committerGuy Harris <[email protected]>
Sat, 9 Jun 2012 02:07:20 +0000 (19:07 -0700)
Make sure we don't run past the end of a BGP attribute or LDP TLV when
dissecting the attribute/TLV.

Make some of the code do a bit more of a "step the pointer through the
data"-style dissection; that was done while debugging the changes in
question.  It also fixes up some code to not check for more data than
should actually be there.

Update references to RFC 4906 from the draft, and note that RFC 4447
replaces it.

decode_prefix.h
print-bgp.c
print-ldp.c

index b73847142f0230a5bce2dfce11c2cf1639600594..8bb4a76750a592f67d8eb700da2a75c6f5e45a5b 100644 (file)
@@ -33,9 +33,9 @@
 #ifndef tcpdump_decode_prefix_h
 #define tcpdump_decode_prefix_h
 
-extern int decode_prefix4(const u_char *pptr, char *buf, u_int buflen);
+extern int decode_prefix4(const u_char *pptr, u_int itemlen, char *buf, u_int buflen);
 #ifdef INET6
-extern int decode_prefix6(const u_char *pd, char *buf, u_int buflen);
+extern int decode_prefix6(const u_char *pd, u_int itemlen, char *buf, u_int buflen);
 #endif
 
 #endif
index 6f35483bd42cb694714e1c0a08a08cbaf6fbd267..dc16d914104b5fb7c29769aa22a72dc97507c7df 100644 (file)
@@ -95,8 +95,6 @@ struct bgp_opt {
 #define BGP_OPT_SIZE           2       /* some compilers may pad to 4 bytes */
 #define BGP_CAP_HEADER_SIZE    2       /* some compilers may pad to 4 bytes */
 
-#define BGP_UPDATE_MINSIZE      23
-
 struct bgp_notification {
        u_int8_t bgpn_marker[16];
        u_int16_t bgpn_len;
@@ -116,19 +114,10 @@ struct bgp_route_refresh {
 };                    /* EXTRACT_16BITS(&bgp_route_refresh->afi) (sigh)      */ 
 #define BGP_ROUTE_REFRESH_SIZE          23
 
-struct bgp_attr {
-       u_int8_t bgpa_flags;
-       u_int8_t bgpa_type;
-       union {
-               u_int8_t len;
-               u_int16_t elen;
-       } bgpa_len;
-#define bgp_attr_len(p) \
-       (((p)->bgpa_flags & 0x10) ? \
-               EXTRACT_16BITS(&(p)->bgpa_len.elen) : (p)->bgpa_len.len)
-#define bgp_attr_off(p) \
-       (((p)->bgpa_flags & 0x10) ? 4 : 3)
-};
+#define bgp_attr_lenlen(flags, p) \
+       (((flags) & 0x10) ? 2 : 1)
+#define bgp_attr_len(flags, p) \
+       (((flags) & 0x10) ? EXTRACT_16BITS(p) : *(p))
 
 #define BGPTYPE_ORIGIN                 1
 #define BGPTYPE_AS_PATH                        2
@@ -494,38 +483,49 @@ as_printf (char *str, int size, u_int asnum)
        return str;
 }
 
+#define ITEMCHECK(minlen) if (itemlen < minlen) goto badtlv;
+
 int
-decode_prefix4(const u_char *pptr, char *buf, u_int buflen)
+decode_prefix4(const u_char *pptr, u_int itemlen, char *buf, u_int buflen)
 {
        struct in_addr addr;
-       u_int plen;
+       u_int plen, plenbytes;
 
        TCHECK(pptr[0]);
+       ITEMCHECK(1);
        plen = pptr[0];
        if (32 < plen)
                return -1;
+       itemlen -= 1;
 
        memset(&addr, 0, sizeof(addr));
-       TCHECK2(pptr[1], (plen + 7) / 8);
-       memcpy(&addr, &pptr[1], (plen + 7) / 8);
+       plenbytes = (plen + 7) / 8;
+       TCHECK2(pptr[1], plenbytes);
+       ITEMCHECK(plenbytes);
+       memcpy(&addr, &pptr[1], plenbytes);
        if (plen % 8) {
-               ((u_char *)&addr)[(plen + 7) / 8 - 1] &=
+               ((u_char *)&addr)[plenbytes - 1] &=
                        ((0xff00 >> (plen % 8)) & 0xff);
        }
        snprintf(buf, buflen, "%s/%d", getname((u_char *)&addr), plen);
-       return 1 + (plen + 7) / 8;
+       return 1 + plenbytes;
 
 trunc:
        return -2;
+
+badtlv:
+       return -3;
 }
 
 static int
-decode_labeled_prefix4(const u_char *pptr, char *buf, u_int buflen)
+decode_labeled_prefix4(const u_char *pptr, u_int itemlen, char *buf, u_int buflen)
 {
        struct in_addr addr;
-       u_int plen;
+       u_int plen, plenbytes;
 
-       TCHECK(pptr[0]);
+       /* prefix length and label = 4 bytes */
+       TCHECK2(pptr[0], 4);
+       ITEMCHECK(4);
        plen = pptr[0];   /* get prefix length */
 
         /* this is one of the weirdnesses of rfc3107
@@ -543,12 +543,15 @@ decode_labeled_prefix4(const u_char *pptr, char *buf, u_int buflen)
 
        if (32 < plen)
                return -1;
+       itemlen -= 4;
 
        memset(&addr, 0, sizeof(addr));
-       TCHECK2(pptr[4], (plen + 7) / 8);
-       memcpy(&addr, &pptr[4], (plen + 7) / 8);
+       plenbytes = (plen + 7) / 8;
+       TCHECK2(pptr[4], plenbytes);
+       ITEMCHECK(plenbytes);
+       memcpy(&addr, &pptr[4], plenbytes);
        if (plen % 8) {
-               ((u_char *)&addr)[(plen + 7) / 8 - 1] &=
+               ((u_char *)&addr)[plenbytes - 1] &=
                        ((0xff00 >> (plen % 8)) & 0xff);
        }
         /* the label may get offsetted by 4 bits so lets shift it right */
@@ -558,10 +561,13 @@ decode_labeled_prefix4(const u_char *pptr, char *buf, u_int buflen)
                  EXTRACT_24BITS(pptr+1)>>4,
                  ((pptr[3]&1)==0) ? "(BOGUS: Bottom of Stack NOT set!)" : "(bottom)" );
 
-       return 4 + (plen + 7) / 8;
+       return 4 + plenbytes;
 
 trunc:
        return -2;
+
+badtlv:
+       return -3;
 }
 
 /*
@@ -1042,37 +1048,46 @@ trunc:
 
 #ifdef INET6
 int
-decode_prefix6(const u_char *pd, char *buf, u_int buflen)
+decode_prefix6(const u_char *pd, u_int itemlen, char *buf, u_int buflen)
 {
        struct in6_addr addr;
-       u_int plen;
+       u_int plen, plenbytes;
 
        TCHECK(pd[0]);
+       ITEMCHECK(1);
        plen = pd[0];
        if (128 < plen)
                return -1;
+       itemlen -= 1;
 
        memset(&addr, 0, sizeof(addr));
-       TCHECK2(pd[1], (plen + 7) / 8);
-       memcpy(&addr, &pd[1], (plen + 7) / 8);
+       plenbytes = (plen + 7) / 8;
+       TCHECK2(pd[1], plenbytes);
+       ITEMCHECK(plenbytes);
+       memcpy(&addr, &pd[1], plenbytes);
        if (plen % 8) {
-               addr.s6_addr[(plen + 7) / 8 - 1] &=
+               addr.s6_addr[plenbytes - 1] &=
                        ((0xff00 >> (plen % 8)) & 0xff);
        }
        snprintf(buf, buflen, "%s/%d", getname6((u_char *)&addr), plen);
-       return 1 + (plen + 7) / 8;
+       return 1 + plenbytes;
 
 trunc:
        return -2;
+
+badtlv:
+       return -3;
 }
 
 static int
-decode_labeled_prefix6(const u_char *pptr, char *buf, u_int buflen)
+decode_labeled_prefix6(const u_char *pptr, u_int itemlen, char *buf, u_int buflen)
 {
        struct in6_addr addr;
-       u_int plen;
+       u_int plen, plenbytes;
 
-       TCHECK(pptr[0]);
+       /* prefix length and label = 4 bytes */
+       TCHECK2(pptr[0], 4);
+       ITEMCHECK(4);
        plen = pptr[0]; /* get prefix length */
 
        if (24 > plen)
@@ -1082,12 +1097,14 @@ decode_labeled_prefix6(const u_char *pptr, char *buf, u_int buflen)
 
        if (128 < plen)
                return -1;
+       itemlen -= 4;
 
        memset(&addr, 0, sizeof(addr));
-       TCHECK2(pptr[4], (plen + 7) / 8);
-       memcpy(&addr, &pptr[4], (plen + 7) / 8);
+       plenbytes = (plen + 7) / 8;
+       TCHECK2(pptr[4], plenbytes);
+       memcpy(&addr, &pptr[4], plenbytes);
        if (plen % 8) {
-               addr.s6_addr[(plen + 7) / 8 - 1] &=
+               addr.s6_addr[plenbytes - 1] &=
                        ((0xff00 >> (plen % 8)) & 0xff);
        }
         /* the label may get offsetted by 4 bits so lets shift it right */
@@ -1097,10 +1114,13 @@ decode_labeled_prefix6(const u_char *pptr, char *buf, u_int buflen)
                  EXTRACT_24BITS(pptr+1)>>4,
                  ((pptr[3]&1)==0) ? "(BOGUS: Bottom of Stack NOT set!)" : "(bottom)" );
 
-       return 4 + (plen + 7) / 8;
+       return 4 + plenbytes;
 
 trunc:
        return -2;
+
+badtlv:
+       return -3;
 }
 
 static int
@@ -1267,7 +1287,7 @@ trunc:
 }
 
 static int
-bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
+bgp_attr_print(u_int atype, const u_char *pptr, u_int len)
 {
        int i;
        u_int16_t af;
@@ -1277,7 +1297,7 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
             u_int32_t i;
         } bw;
        int advance;
-       int tlen;
+       u_int tlen;
        const u_char *tptr;
        char buf[MAXHOSTNAMELEN + 100];
        char tokbuf[TOKBUFSIZE];
@@ -1286,7 +1306,7 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
         tptr = pptr;
         tlen=len;
 
-       switch (attr->bgpa_type) {
+       switch (atype) {
        case BGPTYPE_ORIGIN:
                if (len != 1)
                        printf("invalid len");
@@ -1322,7 +1342,7 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
                  * 2 bytes first, and it does not pass, assume that ASs are
                  * encoded in 4 bytes format and move on.
                  */
-                as_size = bgp_attr_get_as_size(attr->bgpa_type, pptr, len);
+                as_size = bgp_attr_get_as_size(atype, pptr, len);
 
                while (tptr < pptr + len) {
                        TCHECK(tptr[0]);
@@ -1658,20 +1678,24 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
                     case (AFNUM_INET<<8 | SAFNUM_UNICAST):
                     case (AFNUM_INET<<8 | SAFNUM_MULTICAST):
                     case (AFNUM_INET<<8 | SAFNUM_UNIMULTICAST):
-                        advance = decode_prefix4(tptr, buf, sizeof(buf));
+                        advance = decode_prefix4(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
                     case (AFNUM_INET<<8 | SAFNUM_LABUNICAST):
-                        advance = decode_labeled_prefix4(tptr, buf, sizeof(buf));
+                        advance = decode_labeled_prefix4(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
@@ -1719,20 +1743,24 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
                     case (AFNUM_INET6<<8 | SAFNUM_UNICAST):
                     case (AFNUM_INET6<<8 | SAFNUM_MULTICAST):
                     case (AFNUM_INET6<<8 | SAFNUM_UNIMULTICAST):
-                        advance = decode_prefix6(tptr, buf, sizeof(buf));
+                        advance = decode_prefix6(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
                     case (AFNUM_INET6<<8 | SAFNUM_LABUNICAST):
-                        advance = decode_labeled_prefix6(tptr, buf, sizeof(buf));
+                        advance = decode_labeled_prefix6(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
@@ -1822,20 +1850,24 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
                     case (AFNUM_INET<<8 | SAFNUM_UNICAST):
                     case (AFNUM_INET<<8 | SAFNUM_MULTICAST):
                     case (AFNUM_INET<<8 | SAFNUM_UNIMULTICAST):
-                        advance = decode_prefix4(tptr, buf, sizeof(buf));
+                        advance = decode_prefix4(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
                     case (AFNUM_INET<<8 | SAFNUM_LABUNICAST):
-                        advance = decode_labeled_prefix4(tptr, buf, sizeof(buf));
+                        advance = decode_labeled_prefix4(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
@@ -1854,20 +1886,24 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
                     case (AFNUM_INET6<<8 | SAFNUM_UNICAST):
                     case (AFNUM_INET6<<8 | SAFNUM_MULTICAST):
                     case (AFNUM_INET6<<8 | SAFNUM_UNIMULTICAST):
-                        advance = decode_prefix6(tptr, buf, sizeof(buf));
+                        advance = decode_prefix6(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
                     case (AFNUM_INET6<<8 | SAFNUM_LABUNICAST):
-                        advance = decode_labeled_prefix6(tptr, buf, sizeof(buf));
+                        advance = decode_labeled_prefix6(tptr, len, buf, sizeof(buf));
                         if (advance == -1)
                             printf("\n\t    (illegal prefix length)");
                         else if (advance == -2)
                             goto trunc;
+                        else if (advance == -3)
+                            break; /* bytes left, but not enough */
                         else
                             printf("\n\t      %s", buf);
                         break;
@@ -2098,40 +2134,50 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
         }
         case BGPTYPE_ATTR_SET:
                 TCHECK2(tptr[0], 4);
+                if (len < 4)
+                       goto trunc;
                printf("\n\t    Origin AS: %s",
                    as_printf(astostr, sizeof(astostr), EXTRACT_32BITS(tptr)));
                tptr+=4;
                 len -=4;
 
-                while (len >= 2 ) {
-                    int alen;
-                    struct bgp_attr bgpa;
+                while (len) {
+                    u_int aflags, atype, alenlen, alen;
                     
-                    TCHECK2(tptr[0], sizeof(bgpa));
-                    memcpy(&bgpa, tptr, sizeof(bgpa));
-                    alen = bgp_attr_len(&bgpa);
-                    tptr += bgp_attr_off(&bgpa);
-                    len -= bgp_attr_off(&bgpa);
+                    TCHECK2(tptr[0], 2);
+                    if (len < 2)
+                        goto trunc;
+                    aflags = *tptr;
+                    atype = *(tptr + 1);
+                    tptr += 2;
+                    len -= 2;
+                    alenlen = bgp_attr_lenlen(aflags, tptr);
+                    TCHECK2(tptr[0], alenlen);
+                    if (len < alenlen)
+                        goto trunc;
+                    alen = bgp_attr_len(aflags, tptr);
+                    tptr += alenlen;
+                    len -= alenlen;
                     
                     printf("\n\t      %s (%u), length: %u",
                            tok2strbuf(bgp_attr_values,
-                                     "Unknown Attribute", bgpa.bgpa_type,
-                                     tokbuf, sizeof(tokbuf)),
-                           bgpa.bgpa_type,
+                                      "Unknown Attribute", atype,
+                                      tokbuf, sizeof(tokbuf)),
+                           atype,
                            alen);
                     
-                    if (bgpa.bgpa_flags) {
+                    if (aflags) {
                         printf(", Flags [%s%s%s%s",
-                               bgpa.bgpa_flags & 0x80 ? "O" : "",
-                               bgpa.bgpa_flags & 0x40 ? "T" : "",
-                               bgpa.bgpa_flags & 0x20 ? "P" : "",
-                               bgpa.bgpa_flags & 0x10 ? "E" : "");
-                        if (bgpa.bgpa_flags & 0xf)
-                            printf("+%x", bgpa.bgpa_flags & 0xf);
+                               aflags & 0x80 ? "O" : "",
+                               aflags & 0x40 ? "T" : "",
+                               aflags & 0x20 ? "P" : "",
+                               aflags & 0x10 ? "E" : "");
+                        if (aflags & 0xf)
+                            printf("+%x", aflags & 0xf);
                         printf("]: ");
                     }
                     /* FIXME check for recursion */
-                    if (!bgp_attr_print(&bgpa, tptr, alen))
+                    if (!bgp_attr_print(atype, tptr, alen))
                         return 0;
                     tptr += alen;
                     len -= alen;
@@ -2141,7 +2187,7 @@ bgp_attr_print(const struct bgp_attr *attr, const u_char *pptr, int len)
 
        default:
            TCHECK2(*pptr,len);
-            printf("\n\t    no Attribute %u decoder",attr->bgpa_type); /* we have no decoder for the attribute */
+            printf("\n\t    no Attribute %u decoder",atype); /* we have no decoder for the attribute */
             if (vflag <= 1)
                 print_unknown_data(pptr,"\n\t    ",len);
             break;
@@ -2308,107 +2354,162 @@ static void
 bgp_update_print(const u_char *dat, int length)
 {
        struct bgp bgp;
-       struct bgp_attr bgpa;
        const u_char *p;
+       int withdrawn_routes_len;
        int len;
        int i;
        char tokbuf[TOKBUFSIZE];
 
        TCHECK2(dat[0], BGP_SIZE);
+       if (length < BGP_SIZE)
+               goto trunc;
        memcpy(&bgp, dat, BGP_SIZE);
        p = dat + BGP_SIZE;     /*XXX*/
+       length -= BGP_SIZE;
 
        /* Unfeasible routes */
-       len = EXTRACT_16BITS(p);
-       if (len) {
+       TCHECK2(p[0], 2);
+       if (length < 2)
+               goto trunc;
+       withdrawn_routes_len = EXTRACT_16BITS(p);
+       p += 2;
+       length -= 2;
+       if (withdrawn_routes_len) {
                /*
                 * Without keeping state from the original NLRI message,
                 * it's not possible to tell if this a v4 or v6 route,
                 * so only try to decode it if we're not v6 enabled.
                 */
+               TCHECK2(p[0], withdrawn_routes_len);
+               if (length < withdrawn_routes_len)
+                       goto trunc;
 #ifdef INET6
-               printf("\n\t  Withdrawn routes: %d bytes", len);
+               printf("\n\t  Withdrawn routes: %d bytes", withdrawn_routes_len);
+               p += withdrawn_routes_len;
+               length -= withdrawn_routes_len;
 #else
                char buf[MAXHOSTNAMELEN + 100];
                int wpfx;
 
-               TCHECK2(p[2], len);
-               i = 2;
+               if (withdrawn_routes_len < 2)
+                       goto trunc;
+               length -= 2;
+               withdrawn_routes_len -= 2;
+
 
                printf("\n\t  Withdrawn routes:");
 
-               while(i < 2 + len) {
-                       wpfx = decode_prefix4(&p[i], buf, sizeof(buf));
+               while(withdrawn_routes_len > 0) {
+                       wpfx = decode_prefix4(p, withdrawn_routes_len, buf, sizeof(buf));
                        if (wpfx == -1) {
                                printf("\n\t    (illegal prefix length)");
                                break;
                        } else if (wpfx == -2)
                                goto trunc;
+                       } else if (wpfx == -3)
+                               goto trunc; /* bytes left, but not enough */
                        else {
-                               i += wpfx;
                                printf("\n\t    %s", buf);
+                               p += wpfx;
+                               length -= wpfx;
+                               withdrawn_routes_len -= wpfx;
                        }
                }
 #endif
        }
-       p += 2 + len;
 
        TCHECK2(p[0], 2);
+       if (length < 2)
+               goto trunc;
        len = EXTRACT_16BITS(p);
+       p += 2;
+       length -= 2;
 
-        if (len == 0 && length == BGP_UPDATE_MINSIZE) {
+        if (withdrawn_routes_len == 0 && len == 0 && length == 0) {
+            /* No withdrawn routes, no path attributes, no NLRI */
             printf("\n\t  End-of-Rib Marker (empty NLRI)");
             return;
         }
 
        if (len) {
                /* do something more useful!*/
-               i = 2;
-               while (i < 2 + len) {
-                       int alen, aoff;
-
-                       TCHECK2(p[i], sizeof(bgpa));
-                       memcpy(&bgpa, &p[i], sizeof(bgpa));
-                       alen = bgp_attr_len(&bgpa);
-                       aoff = bgp_attr_off(&bgpa);
-
-                      printf("\n\t  %s (%u), length: %u",
+               while (len) {
+                       int aflags, atype, alenlen, alen;
+
+                       TCHECK2(p[0], 2);
+                       if (len < 2)
+                           goto trunc;
+                       if (length < 2)
+                           goto trunc;
+                       aflags = *p;
+                       atype = *(p + 1);
+                       p += 2;
+                       len -= 2;
+                       length -= 2;
+                       alenlen = bgp_attr_lenlen(aflags, p);
+                       TCHECK2(p[0], alenlen);
+                       if (len < alenlen)
+                           goto trunc;
+                       if (length < alenlen)
+                           goto trunc;
+                       alen = bgp_attr_len(aflags, p);
+                       p += alenlen;
+                       len -= alenlen;
+                       length -= alenlen;
+
+                       printf("\n\t  %s (%u), length: %u",
                               tok2strbuf(bgp_attr_values, "Unknown Attribute",
-                                        bgpa.bgpa_type,
+                                        atype,
                                         tokbuf, sizeof(tokbuf)),
-                              bgpa.bgpa_type,
+                              atype,
                               alen);
 
-                       if (bgpa.bgpa_flags) {
+                       if (aflags) {
                                printf(", Flags [%s%s%s%s",
-                                       bgpa.bgpa_flags & 0x80 ? "O" : "",
-                                       bgpa.bgpa_flags & 0x40 ? "T" : "",
-                                       bgpa.bgpa_flags & 0x20 ? "P" : "",
-                                       bgpa.bgpa_flags & 0x10 ? "E" : "");
-                               if (bgpa.bgpa_flags & 0xf)
-                                       printf("+%x", bgpa.bgpa_flags & 0xf);
+                                       aflags & 0x80 ? "O" : "",
+                                       aflags & 0x40 ? "T" : "",
+                                       aflags & 0x20 ? "P" : "",
+                                       aflags & 0x10 ? "E" : "");
+                               if (aflags & 0xf)
+                                       printf("+%x", aflags & 0xf);
                                printf("]: ");
                        }
-                       if (!bgp_attr_print(&bgpa, &p[i + aoff], alen))
+                       if (len < alen)
+                               goto trunc;
+                       if (length < alen)
                                goto trunc;
-                       i += aoff + alen;
+                       if (!bgp_attr_print(atype, p, alen))
+                               goto trunc;
+                       p += alen;
+                       len -= alen;
+                       length -= alen;
                }
        } 
-       p += 2 + len;
 
-       if (dat + length > p) {
+       if (length) {
+               /*
+                * XXX - what if they're using the "Advertisement of
+                * Multiple Paths in BGP" feature:
+                *
+                * https://datatracker.ietf.org/doc/draft-ietf-idr-add-paths/
+                *
+                * http://tools.ietf.org/html/draft-ietf-idr-add-paths-06
+                */
                printf("\n\t  Updated routes:");
-               while (dat + length > p) {
+               while (length) {
                        char buf[MAXHOSTNAMELEN + 100];
-                       i = decode_prefix4(p, buf, sizeof(buf));
+                       i = decode_prefix4(p, length, buf, sizeof(buf));
                        if (i == -1) {
                                printf("\n\t    (illegal prefix length)");
                                break;
                        } else if (i == -2)
                                goto trunc;
+                       else if (i == -3)
+                               goto trunc; /* bytes left, but not enough */
                        else {
                                printf("\n\t    %s", buf);
                                p += i;
+                               length -= i;
                        }
                }
        }
index 1243104855b7414faaca60c2623b21695192199e..262c9bda71b66e1ecb92f70e0b13006d08b9436a 100644 (file)
@@ -180,7 +180,7 @@ static const struct tok ldp_tlv_values[] = {
 #define LDP_FEC_WILDCARD       0x01
 #define LDP_FEC_PREFIX         0x02
 #define LDP_FEC_HOSTADDRESS    0x03
-/* From draft-martini-l2circuit-trans-mpls-13.txt */
+/* From RFC 4906; should probably be updated to RFC 4447 (e.g., VC -> PW) */
 #define LDP_FEC_MARTINI_VC     0x80
 
 static const struct tok ldp_fec_values[] = {
@@ -238,6 +238,9 @@ int ldp_tlv_print(register const u_char *);
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  */
 
+#define TLV_TCHECK(minlen) \
+    TCHECK2(*tptr, minlen); if (tlv_tlen < minlen) goto badtlv;
+
 int
 ldp_tlv_print(register const u_char *tptr) {
 
@@ -273,6 +276,7 @@ ldp_tlv_print(register const u_char *tptr) {
     switch(tlv_type) {
 
     case LDP_TLV_COMMON_HELLO:
+        TLV_TCHECK(4);
         printf("\n\t      Hold Time: %us, Flags: [%s Hello%s]",
                EXTRACT_16BITS(tptr),
                (EXTRACT_16BITS(tptr+2)&0x8000) ? "Targeted" : "Link",
@@ -280,18 +284,22 @@ ldp_tlv_print(register const u_char *tptr) {
         break;
 
     case LDP_TLV_IPV4_TRANSPORT_ADDR:
+        TLV_TCHECK(4);
         printf("\n\t      IPv4 Transport Address: %s", ipaddr_string(tptr));
         break;
 #ifdef INET6
     case LDP_TLV_IPV6_TRANSPORT_ADDR:
+        TLV_TCHECK(16);
         printf("\n\t      IPv6 Transport Address: %s", ip6addr_string(tptr));
         break;
 #endif
     case LDP_TLV_CONFIG_SEQ_NUMBER:
+        TLV_TCHECK(4);
         printf("\n\t      Sequence Number: %u", EXTRACT_32BITS(tptr));
         break;
 
     case LDP_TLV_ADDRESS_LIST:
+        TLV_TCHECK(LDP_TLV_ADDRESS_LIST_AFNUM_LEN);
        af = EXTRACT_16BITS(tptr);
        tptr+=LDP_TLV_ADDRESS_LIST_AFNUM_LEN;
         tlv_tlen -= LDP_TLV_ADDRESS_LIST_AFNUM_LEN;
@@ -300,6 +308,7 @@ ldp_tlv_print(register const u_char *tptr) {
         switch (af) {
         case AFNUM_INET:
            while(tlv_tlen >= sizeof(struct in_addr)) {
+               TCHECK2(*tptr, sizeof(struct in_addr));
                printf(" %s",ipaddr_string(tptr));
                tlv_tlen-=sizeof(struct in_addr);
                tptr+=sizeof(struct in_addr);                
@@ -308,6 +317,7 @@ ldp_tlv_print(register const u_char *tptr) {
 #ifdef INET6
         case AFNUM_INET6:
            while(tlv_tlen >= sizeof(struct in6_addr)) {
+               TCHECK2(*tptr, sizeof(struct in6_addr));
                printf(" %s",ip6addr_string(tptr));
                tlv_tlen-=sizeof(struct in6_addr);
                tptr+=sizeof(struct in6_addr);                
@@ -321,6 +331,7 @@ ldp_tlv_print(register const u_char *tptr) {
        break;
 
     case LDP_TLV_COMMON_SESSION:
+       TLV_TCHECK(8);
        printf("\n\t      Version: %u, Keepalive: %us, Flags: [Downstream %s, Loop Detection %s]",
               EXTRACT_16BITS(tptr), EXTRACT_16BITS(tptr+2),
               (EXTRACT_16BITS(tptr+6)&0x8000) ? "On Demand" : "Unsolicited",
@@ -329,50 +340,86 @@ ldp_tlv_print(register const u_char *tptr) {
        break;
 
     case LDP_TLV_FEC:
+        TLV_TCHECK(1);
         fec_type = *tptr;
        printf("\n\t      %s FEC (0x%02x)",
               tok2str(ldp_fec_values, "Unknown", fec_type),
               fec_type);
 
        tptr+=1;
+       tlv_tlen-=1;
        switch(fec_type) {
 
        case LDP_FEC_WILDCARD:
            break;
        case LDP_FEC_PREFIX:
+           TLV_TCHECK(2);
            af = EXTRACT_16BITS(tptr);
-           tptr+=2;
+           tptr+=LDP_TLV_ADDRESS_LIST_AFNUM_LEN;
+           tlv_tlen-=LDP_TLV_ADDRESS_LIST_AFNUM_LEN;
            if (af == AFNUM_INET) {
-               i=decode_prefix4(tptr,buf,sizeof(buf));
-               printf(": IPv4 prefix %s",buf);
+               i=decode_prefix4(tptr,tlv_tlen,buf,sizeof(buf));
+               if (i == -2)
+                   goto trunc;
+               if (i == -3)
+                   printf(": IPv4 prefix (goes past end of TLV)");
+               else if (i == -1)
+                   printf(": IPv4 prefix (invalid length)");
+               else
+                   printf(": IPv4 prefix %s",buf);
            }
 #ifdef INET6
            else if (af == AFNUM_INET6) {
-               i=decode_prefix6(tptr,buf,sizeof(buf));
-               printf(": IPv6 prefix %s",buf);
+               i=decode_prefix6(tptr,tlv_tlen,buf,sizeof(buf));
+               if (i == -2)
+                   goto trunc;
+               if (i == -3)
+                   printf(": IPv4 prefix (goes past end of TLV)");
+               else if (i == -1)
+                   printf(": IPv6 prefix (invalid length)");
+               else
+                   printf(": IPv6 prefix %s",buf);
            }
 #endif
+           else
+               printf(": Address family %u prefix", af);
            break;
        case LDP_FEC_HOSTADDRESS:
            break;
        case LDP_FEC_MARTINI_VC:
-            if (!TTEST2(*tptr, 11))
-                goto trunc;
+            /*
+            * According to RFC 4908, the VC info Length field can be zero,
+            * in which case not only are there no interface parameters,
+            * there's no VC ID.
+            */
+            TLV_TCHECK(7);
             vc_info_len = *(tptr+2);
 
+            if (vc_info_len == 0) {
+                printf(": %s, %scontrol word, group-ID %u, VC-info-length: %u",
+                       tok2str(l2vpn_encaps_values, "Unknown", EXTRACT_16BITS(tptr)&0x7fff),
+                       EXTRACT_16BITS(tptr)&0x8000 ? "" : "no ",
+                       EXTRACT_32BITS(tptr+3),
+                       vc_info_len);
+                break;
+            }
+
+            /* Make sure we have the VC ID as well */
+            TLV_TCHECK(11);
            printf(": %s, %scontrol word, group-ID %u, VC-ID %u, VC-info-length: %u",
                   tok2str(l2vpn_encaps_values, "Unknown", EXTRACT_16BITS(tptr)&0x7fff),
                   EXTRACT_16BITS(tptr)&0x8000 ? "" : "no ",
                    EXTRACT_32BITS(tptr+3),
                   EXTRACT_32BITS(tptr+7),
                    vc_info_len);
+            if (vc_info_len < 4)
+                goto trunc; /* minimum 4, for the VC ID */
+            vc_info_len -= 4; /* subtract out the VC ID, giving the length of the interface parameters */
 
-            if (vc_info_len == 0) /* infinite loop protection */
-                break;
-
+            /* Skip past the fixed information and the VC ID */
             tptr+=11;
-            if (!TTEST2(*tptr, vc_info_len))
-                goto trunc;
+            tlv_tlen-=11;
+            TLV_TCHECK(vc_info_len);
 
             while (vc_info_len > 2) {
                 vc_info_tlv_type = *tptr;
@@ -421,10 +468,12 @@ ldp_tlv_print(register const u_char *tptr) {
        break;
 
     case LDP_TLV_GENERIC_LABEL:
+       TLV_TCHECK(4);
        printf("\n\t      Label: %u", EXTRACT_32BITS(tptr) & 0xfffff);
        break;
 
     case LDP_TLV_STATUS:
+       TLV_TCHECK(8);
        ui = EXTRACT_32BITS(tptr);
        tptr+=4;
        printf("\n\t      Status: 0x%02x, Flags: [%s and %s forward]",
@@ -438,6 +487,7 @@ ldp_tlv_print(register const u_char *tptr) {
        break;
 
     case LDP_TLV_FT_SESSION:
+       TLV_TCHECK(8);
        ft_flags = EXTRACT_16BITS(tptr);
        printf("\n\t      Flags: [%sReconnect, %sSave State, %sAll-Label Protection, %s Checkpoint, %sRe-Learn State]",
               ft_flags&0x8000 ? "" : "No ",
@@ -456,6 +506,7 @@ ldp_tlv_print(register const u_char *tptr) {
        break;
 
     case LDP_TLV_MTU:
+       TLV_TCHECK(2);
        printf("\n\t      MTU: %u", EXTRACT_16BITS(tptr));
        break;
 
@@ -486,6 +537,10 @@ ldp_tlv_print(register const u_char *tptr) {
 trunc:
     printf("\n\t\t packet exceeded snapshot");
     return 0;
+
+badtlv:
+    printf("\n\t\t TLV contents go past end of TLV");
+    return(tlv_len+4); /* Type & Length fields not included */
 }
 
 void
@@ -546,8 +601,7 @@ ldp_msg_print(register const u_char *pptr) {
 
     while(tlen>0) {
         /* did we capture enough for fully decoding the msg header ? */
-        if (!TTEST2(*tptr, sizeof(struct ldp_msg_header)))
-            goto trunc;
+        TCHECK2(*tptr, sizeof(struct ldp_msg_header));
 
         ldp_msg_header = (const struct ldp_msg_header *)tptr;
         msg_len=EXTRACT_16BITS(ldp_msg_header->length);
@@ -570,8 +624,7 @@ ldp_msg_print(register const u_char *pptr) {
         msg_tlen=msg_len-sizeof(struct ldp_msg_header)+4; /* Type & Length fields not included */
 
         /* did we capture enough for fully decoding the message ? */
-        if (!TTEST2(*tptr, msg_len))
-            goto trunc;
+        TCHECK2(*tptr, msg_len);
         hexdump=FALSE;
 
         switch(msg_type) {