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 c401d0049cc602dcd9640ca52d1d70f090ba094b..18b64cb03d7dda74840b69c1c6299997327d9a8b 100644 (file)
--- 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 473e96f4bfa7ddb02189d565e07c119edb161d11..c62c8d52f157152a1df46d76b4d108df1d7c227e 100644 (file)
--- 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 00319b729968770ee9fb616347d4e7cb82ff3ef6..777ceece290260d50fab8bf6613f1ac437199b57 100644 (file)
--- 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 c06029f7b0af03d635f6f6532a6daa9b13e09b6a..9a4c2bda6e70cd9f95a7e53f88af0555f7856eda 100644 (file)
--- 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