IS-IS: Fix some loops for undefined behavior at runtime

Same bugfixes than in previous commit, based on a code inspection.

Update the output of a test accordingly.

Moreover:
Clean up whitespaces/indentation.

diff --git a/print-isoclns.c b/print-isoclns.c
index b2cc946701fdafa85e059f8a75601842cdd76174..361497ae4b18823ac7acd574df43c207b6d98b3e 100644 (file)
--- a/print-isoclns.c
+++ b/print-isoclns.c
@@ -2724,27 +2724,41 @@ isis_print(netdissect_options *ndo,
                 ext_is_len = isis_print_ext_is_reach(ndo, tptr, "\n\t      ", tlv_type, tlen);
                 if (ext_is_len == 0) /* did something go wrong ? */
                     goto trunc;
-
+                if (tlen < ext_is_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_is_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
                 tlen-=ext_is_len;
                 tptr+=ext_is_len;
             }
             break;
 
         case ISIS_TLV_IS_ALIAS_ID:
-           while (tlen >= NODE_ID_LEN+1) { /* is it worth attempting a decode ? */
-               ext_is_len = isis_print_ext_is_reach(ndo, tptr, "\n\t      ", tlv_type, tlen);
-               if (ext_is_len == 0) /* did something go wrong ? */
-                   goto trunc;
-               tlen-=ext_is_len;
-               tptr+=ext_is_len;
-           }
-           break;
+            while (tlen >= NODE_ID_LEN+1) { /* is it worth attempting a decode ? */
+                ext_is_len = isis_print_ext_is_reach(ndo, tptr, "\n\t      ", tlv_type, tlen);
+                if (ext_is_len == 0) /* did something go wrong ? */
+                    goto trunc;
+                if (tlen < ext_is_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_is_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
+                tlen-=ext_is_len;
+                tptr+=ext_is_len;
+            }
+            break;
 
         case ISIS_TLV_EXT_IS_REACH:
             while (tlen >= NODE_ID_LEN+3+1) { /* is it worth attempting a decode ? */
                 ext_is_len = isis_print_ext_is_reach(ndo, tptr, "\n\t      ", tlv_type, tlen);
                 if (ext_is_len == 0) /* did something go wrong ? */
                     goto trunc;
+                if (tlen < ext_is_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_is_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
                 tlen-=ext_is_len;
                 tptr+=ext_is_len;
             }
@@ -2787,14 +2801,19 @@ isis_print(netdissect_options *ndo,
                break;
 
        case ISIS_TLV_EXTD_IP_REACH:
-           while (tlen>0) {
+            while (tlen != 0) {
                 ext_ip_len = isis_print_extd_ip_reach(ndo, tptr, "\n\t      ", AF_INET);
                 if (ext_ip_len == 0) /* did something go wrong ? */
                     goto trunc;
+                if (tlen < ext_ip_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_ip_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
                 tptr+=ext_ip_len;
-               tlen-=ext_ip_len;
-           }
-           break;
+                tlen-=ext_ip_len;
+            }
+            break;
 
         case ISIS_TLV_MT_IP_REACH:
             mt_len = isis_print_mtid(ndo, tptr, "\n\t      ");
@@ -2804,14 +2823,19 @@ isis_print(netdissect_options *ndo,
             tptr+=mt_len;
             tlen-=mt_len;
 
-            while (tlen>0) {
+            while (tlen != 0) {
                 ext_ip_len = isis_print_extd_ip_reach(ndo, tptr, "\n\t      ", AF_INET);
                 if (ext_ip_len == 0) /* did something go wrong ? */
                     goto trunc;
+                if (tlen < ext_ip_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_ip_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
                 tptr+=ext_ip_len;
-               tlen-=ext_ip_len;
-           }
-           break;
+                tlen-=ext_ip_len;
+            }
+            break;
 
        case ISIS_TLV_IP6_REACH:
             while (tlen != 0) {
@@ -2836,14 +2860,19 @@ isis_print(netdissect_options *ndo,
             tptr+=mt_len;
             tlen-=mt_len;
 
-           while (tlen>0) {
+            while (tlen != 0) {
                 ext_ip_len = isis_print_extd_ip_reach(ndo, tptr, "\n\t      ", AF_INET6);
                 if (ext_ip_len == 0) /* did something go wrong ? */
                     goto trunc;
+                if (tlen < ext_ip_len) {
+                    ND_PRINT(" [remaining tlv length %u < %u]", tlen, ext_ip_len);
+                    nd_print_invalid(ndo);
+                    break;
+                }
                 tptr+=ext_ip_len;
-               tlen-=ext_ip_len;
-           }
-           break;
+                tlen-=ext_ip_len;
+            }
+            break;
 
        case ISIS_TLV_IP6ADDR:
            while (tlen>=sizeof(nd_ipv6)) {

diff --git a/tests/isis-seg-fault-1-v.out b/tests/isis-seg-fault-1-v.out
index bd05c3d0ab609fedd834fad37e2b89a87e573c25..f88a09a8aa4469a5c4dce81e1bb7dd9206036d64 100644 (file)
--- a/tests/isis-seg-fault-1-v.out
+++ b/tests/isis-seg-fault-1-v.out
@@ -42,70 +42,7 @@
              IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
              IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
              IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.7d, no sub-TLVs present
-             IS Neighbor: 0000.4a00.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.80, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0034.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, sub-TLVs present (35)
-               unknown subTLV #2, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #105, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #0, length: 0
-               unknown subTLV #2, length: 0
-               unknown subTLV #0, length: 0
-               Remaining data in subTLVs shorter than a subTLV header
-             IS Neighbor: 0000.0000.0000.67, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.6500.8000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.4200.0000.00, sub-TLVs present (8)
-               unknown subTLV #0, length: 0
-               Remaining data in TLV shorter than a subTLV header
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 7901.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.32, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0800.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0089.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0025.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0025.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0025.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0800.00, no sub-TLVs present
-             IS Neighbor: 2500.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 4a00.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
-             IS Neighbor: 0000.0000.0000.00, no sub-TLVs present
+             IS Neighbor: 0000.0000.0000.7d, no sub-TLVs present [remaining tlv length 9 < 11] (invalid)
            Padding TLV #8, length: 255
            Padding TLV #8, length: 255
            Padding TLV #8, length: 247