Strengthen various bounds etc. checks.

Add more checks, make some checks do a better job of handling too-short
lengths,

Also, rename ldp_msg_print() to ldp_pdu_print(), as it prints a single
PDU, not a single message within a PDU.

1 files changed, 42 insertions, 17 deletions

diff --git a/print-ldp.c b/print-ldp.c
index fc5ff42e..f387affd 100644
--- a/print-ldp.c
+++ b/print-ldp.c
@@ -209,7 +209,7 @@ static const struct tok ldp_fec_martini_ifparm_vccv_cv_values[] = {
     { 0, NULL}
 };
 
-static int ldp_msg_print(netdissect_options *, register const u_char *);
+static int ldp_pdu_print(netdissect_options *, register const u_char *);
 
 /*
  * ldp tlv header
@@ -233,7 +233,8 @@ static int ldp_msg_print(netdissect_options *, register const u_char *);
 
 static int
 ldp_tlv_print(netdissect_options *ndo,
-              register const u_char *tptr) {
+              register const u_char *tptr,
+              u_short msg_tlen) {
 
     struct ldp_tlv_header {
         uint8_t type[2];
@@ -248,7 +249,12 @@ ldp_tlv_print(netdissect_options *ndo,
     int i;
 
     ldp_tlv_header = (const struct ldp_tlv_header *)tptr;
+    ND_TCHECK(*ldp_tlv_header);
     tlv_len=EXTRACT_16BITS(ldp_tlv_header->length);
+    if (tlv_len + 4 > msg_tlen) {
+        ND_PRINT((ndo, "\n\t\t TLV contents go past end of message"));
+        return 0;
+    }
     tlv_tlen=tlv_len;
     tlv_type=LDP_MASK_TLV_TYPE(EXTRACT_16BITS(ldp_tlv_header->type));
 
@@ -403,8 +409,11 @@ ldp_tlv_print(netdissect_options *ndo,
                    EXTRACT_32BITS(tptr+3),
 		   EXTRACT_32BITS(tptr+7),
                    vc_info_len));
-            if (vc_info_len < 4)
-                goto trunc; /* minimum 4, for the VC ID */
+            if (vc_info_len < 4) {
+                /* minimum 4, for the VC ID */
+                ND_PRINT((ndo, " (invalid, < 4"));
+                return(tlv_len+4); /* Type & Length fields not included */
+	    }
             vc_info_len -= 4; /* subtract out the VC ID, giving the length of the interface parameters */
 
             /* Skip past the fixed information and the VC ID */
@@ -540,7 +549,7 @@ ldp_print(netdissect_options *ndo,
 
     int processed;
     while (len > (sizeof(struct ldp_common_header) + sizeof(struct ldp_msg_header))) {
-        processed = ldp_msg_print(ndo, pptr);
+        processed = ldp_pdu_print(ndo, pptr);
         if (processed == 0)
             return;
         len -= processed;
@@ -549,7 +558,7 @@ ldp_print(netdissect_options *ndo,
 }
 
 static int
-ldp_msg_print(netdissect_options *ndo,
+ldp_pdu_print(netdissect_options *ndo,
               register const u_char *pptr) {
 
     const struct ldp_common_header *ldp_com_header;
@@ -559,7 +568,6 @@ ldp_msg_print(netdissect_options *ndo,
     u_short pdu_len,msg_len,msg_type,msg_tlen;
     int hexdump,processed;
 
-    tptr=pptr;
     ldp_com_header = (const struct ldp_common_header *)pptr;
     ND_TCHECK(*ldp_com_header);
 
@@ -573,8 +581,17 @@ ldp_msg_print(netdissect_options *ndo,
 	return 0;
     }
 
-    /* print the LSR-ID, label-space & length */
     pdu_len = EXTRACT_16BITS(&ldp_com_header->pdu_length);
+    if (pdu_len < sizeof(const struct ldp_common_header)-4) {
+        /* length too short */
+        ND_PRINT((ndo, "%sLDP, pdu-length: %u (too short, < %u)",
+               (ndo->ndo_vflag < 1) ? "" : "\n\t",
+               pdu_len,
+               (u_int)(sizeof(const struct ldp_common_header)-4)));
+        return 0;
+    }
+
+    /* print the LSR-ID, label-space & length */
     ND_PRINT((ndo, "%sLDP, Label-Space-ID: %s:%u, pdu-length: %u",
            (ndo->ndo_vflag < 1) ? "" : "\n\t",
            ipaddr_string(ndo, &ldp_com_header->lsr_id),
@@ -586,10 +603,8 @@ ldp_msg_print(netdissect_options *ndo,
         return 0;
 
     /* ok they seem to want to know everything - lets fully decode it */
-    tlen=pdu_len;
-
-    tptr += sizeof(const struct ldp_common_header);
-    tlen -= sizeof(const struct ldp_common_header)-4;	/* Type & Length fields not included */
+    tptr = pptr + sizeof(const struct ldp_common_header);
+    tlen = pdu_len - (sizeof(const struct ldp_common_header)-4);	/* Type & Length fields not included */
 
     while(tlen>0) {
         /* did we capture enough for fully decoding the msg header ? */
@@ -599,6 +614,19 @@ ldp_msg_print(netdissect_options *ndo,
         msg_len=EXTRACT_16BITS(ldp_msg_header->length);
         msg_type=LDP_MASK_MSG_TYPE(EXTRACT_16BITS(ldp_msg_header->type));
 
+        if (msg_len < sizeof(struct ldp_msg_header)-4) {
+            /* length too short */
+            /* FIXME vendor private / experimental check */
+            ND_PRINT((ndo, "\n\t  %s Message (0x%04x), length: %u (too short, < %u)",
+                   tok2str(ldp_msg_values,
+                           "Unknown",
+                           msg_type),
+                   msg_type,
+                   msg_len,
+                   (u_int)(sizeof(struct ldp_msg_header)-4)));
+            return 0;
+        }
+
         /* FIXME vendor private / experimental check */
         ND_PRINT((ndo, "\n\t  %s Message (0x%04x), length: %u, Message ID: 0x%08x, Flags: [%s if unknown]",
                tok2str(ldp_msg_values,
@@ -609,11 +637,8 @@ ldp_msg_print(netdissect_options *ndo,
                EXTRACT_32BITS(&ldp_msg_header->id),
                LDP_MASK_U_BIT(EXTRACT_16BITS(&ldp_msg_header->type)) ? "continue processing" : "ignore"));
 
-        if (msg_len == 0) /* infinite loop protection */
-            return 0;
-
         msg_tptr=tptr+sizeof(struct ldp_msg_header);
-        msg_tlen=msg_len-sizeof(struct ldp_msg_header)+4; /* Type & Length fields not included */
+        msg_tlen=msg_len-(sizeof(struct ldp_msg_header)-4); /* Type & Length fields not included */
 
         /* did we capture enough for fully decoding the message ? */
         ND_TCHECK2(*tptr, msg_len);
@@ -630,7 +655,7 @@ ldp_msg_print(netdissect_options *ndo,
         case LDP_MSG_ADDRESS_WITHDRAW:
         case LDP_MSG_LABEL_WITHDRAW:
             while(msg_tlen >= 4) {
-                processed = ldp_tlv_print(ndo, msg_tptr);
+                processed = ldp_tlv_print(ndo, msg_tptr, msg_tlen);
                 if (processed == 0)
                     break;
                 msg_tlen-=processed;