From: Francois-Xavier Le Bail Date: Mon, 8 Apr 2019 17:03:36 +0000 (+0200) Subject: IS-IS: Fix some loops for undefined behavior at runtime X-Git-Tag: tcpdump-4.99-bp~846 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/c36abfe8a86fb2e3d8256fa53d6569a4aa65b39c 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 b2cc9467..361497ae 100644 --- 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 bd05c3d0..f88a09a8 100644 --- 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