From: Guy Harris Date: Sun, 31 Dec 2017 09:49:47 +0000 (-0800) Subject: Don't treat going past the end of the buffer in check_add_path() as an error. X-Git-Tag: tcpdump-4.99-bp~1516 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/57ad83491add64cb7e3eaae0ba5205b75b3a5253 Don't treat going past the end of the buffer in check_add_path() as an error. It's just a heuristic test to try to guess whether the NLRI information has path IDs or not. If we run past the end of the packet data, just stop processing before we try to fetch data past the end, don't return an error. This keeps us from showing truncation in non-truncated packets. Update tests to reflect that change. --- diff --git a/print-bgp.c b/print-bgp.c index c401d004..18b64cb0 100644 --- a/print-bgp.c +++ b/print-bgp.c @@ -32,7 +32,7 @@ /* \summary: Border Gateway Protocol (BGP) printer */ -/* specification: RFC 4771 */ +/* specification: RFC 4271 */ #ifdef HAVE_CONFIG_H #include "config.h" @@ -1405,10 +1405,16 @@ check_add_path(netdissect_options *ndo, const u_char *pptr, u_int length, return 0; } - /* check if it could be add path */ + /* + * Scan through the NLRI information under the assumpetion that + * it doesn't have path IDs. + */ for (offset = 0; offset < length;) { offset += 4; - ND_TCHECK_1(pptr + offset); + if (!ND_TTEST_1(pptr + offset)) { + /* We ran out of captured data; quit scanning. */ + break; + } prefix_length = EXTRACT_U_1(pptr + offset); /* * Add 4 to cover the path id @@ -1427,7 +1433,10 @@ check_add_path(netdissect_options *ndo, const u_char *pptr, u_int length, /* check it's not standard BGP */ for (offset = 0; offset < length; ) { - ND_TCHECK_1(pptr + offset); + if (!ND_TTEST_1(pptr + offset)) { + /* We ran out of captured data; quit scanning. */ + break; + } prefix_length = EXTRACT_U_1(pptr + offset); /* * If the prefix_length is zero (0.0.0.0/0) @@ -1445,8 +1454,6 @@ check_add_path(netdissect_options *ndo, const u_char *pptr, u_int length, /* assume not add-path by default */ return 0; -trunc: - return -1; } static int @@ -1838,11 +1845,7 @@ bgp_attr_print(netdissect_options *ndo, } add_path4 = check_add_path(ndo, tptr, (len-(tptr - pptr)), 32); - if (add_path4 == -1) - goto trunc; add_path6 = check_add_path(ndo, tptr, (len-(tptr - pptr)), 128); - if (add_path6 == -1) - goto trunc; while (tptr < pptr + len) { switch (af<<8 | safi) { @@ -2027,11 +2030,7 @@ bgp_attr_print(netdissect_options *ndo, tptr += 3; add_path4 = check_add_path(ndo, tptr, (len-(tptr - pptr)), 32); - if (add_path4 == -1) - goto trunc; add_path6 = check_add_path(ndo, tptr, (len-(tptr - pptr)), 128); - if (add_path6 == -1) - goto trunc; while (tptr < pptr + len) { switch (af<<8 | safi) { @@ -2679,8 +2678,6 @@ bgp_update_print(netdissect_options *ndo, goto trunc; ND_PRINT((ndo, "\n\t Withdrawn routes:")); add_path = check_add_path(ndo, p, withdrawn_routes_len, 32); - if (add_path == -1) - goto trunc; while (withdrawn_routes_len != 0) { if (add_path) { if (withdrawn_routes_len < 4) { @@ -2784,8 +2781,6 @@ bgp_update_print(netdissect_options *ndo, if (length) { add_path = check_add_path(ndo, p, length, 32); - if (add_path == -1) - goto trunc; ND_PRINT((ndo, "\n\t Updated routes:")); while (length != 0) { if (add_path) { diff --git a/tests/bgp_vpn_attrset.out b/tests/bgp_vpn_attrset.out index 473e96f4..c62c8d52 100644 --- a/tests/bgp_vpn_attrset.out +++ b/tests/bgp_vpn_attrset.out @@ -15,4 +15,5 @@ IP (tos 0xc0, ttl 62, id 58628, offset 0, flags [none], proto TCP (6), length 17 Cluster List (10), length: 4, Flags [O]: 22.5.5.5 Multi-Protocol Reach NLRI (14), length: 30, Flags [OE]: AFI: IPv4 (1), SAFI: labeled VPN Unicast (128) - nexthop: RD: 0:0 (= 0.0.0.0), 12.4.4.4, nh-length: 12, no SNPA[|BGP] + nexthop: RD: 0:0 (= 0.0.0.0), 12.4.4.4, nh-length: 12, no SNPA + RD: 500:500 (= 0.0.1.244), 133.0.0.0/8, label:100208 (bottom) diff --git a/tests/bgp_vpn_rt-oobr.out b/tests/bgp_vpn_rt-oobr.out index 00319b72..777ceece 100644 --- a/tests/bgp_vpn_rt-oobr.out +++ b/tests/bgp_vpn_rt-oobr.out @@ -20,4 +20,26 @@ IP (tos 0xc, ttl 254, id 21263, offset 0, flags [rsvd], proto TCP (6), length 60 Origin AS: 0 Multi-Protocol Reach NLRI (14), length: 227, Flags [T+6]: AFI: IPv4 (1), vendor specific SAFI: Route Target Routing Information (132) - nexthop: invalid len, nh-length: 1, no SNPA[|BGP] + nexthop: invalid len, nh-length: 1, no SNPA + origin AS: 0, route target 0:0 (= 0.0.0.0) + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target + default route target[|BGP] diff --git a/tests/mpbgp-linklocal-nexthop.out b/tests/mpbgp-linklocal-nexthop.out index c06029f7..9a4c2bda 100644 --- a/tests/mpbgp-linklocal-nexthop.out +++ b/tests/mpbgp-linklocal-nexthop.out @@ -6,4 +6,5 @@ IP (tos 0xc0, ttl 64, id 22725, offset 0, flags [DF], proto TCP (6), length 142) Next Hop (3), length: 4, Flags [T]: 0.0.0.0 Multi-Protocol Reach NLRI (14), length: 46, Flags [O]: AFI: IPv6 (2), SAFI: Unicast (1) - nexthop: dead:beef::1, fe80::1ff:fe01:0, nh-length: 32, no SNPA[|BGP] + nexthop: dead:beef::1, fe80::1ff:fe01:0, nh-length: 32, no SNPA + 4:5::/64