Add some attribute/TLV length checks.

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.

1 files changed, 70 insertions, 17 deletions

diff --git a/print-ldp.c b/print-ldp.c
index 12431048..262c9bda 100644
--- a/print-ldp.c
+++ b/print-ldp.c
@@ -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) {