RIP: Modernize packet parsing style.

Enable ND_LONGJMP_FROM_TCHECK. Report invalid packets as invalid. Remove
two redundant ND_TCHECK_SIZE() instances and an improvised snapshot end
guard. Check bounds for the remaining part of the packet header after
printing version and command, not before. Lose one pointer and one
length variable in rip_print(), also account for the header size when
estimating the number of routes. Update two tests.

diff --git a/CHANGES b/CHANGES
index e22bd5081b42d3441761072f038b7b07560832c9..69892c96b1ed7428a36201ef98dfb04ef96e7908 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,7 +2,7 @@ Monthday, Month DD, YYYY by gharris and denis
   Summary for 5.0.0 tcpdump release (so far!)
     Source code:
       Use %zu when printing a sizeof to squelch compiler warnings
-      AppleTalk, BOOTP, EGP, EIGRP, Geneve, L2TP, OLSR, PGM, RSVP, UDP: Modernize packet parsing style
+      AppleTalk, BOOTP, EGP, EIGRP, Geneve, L2TP, OLSR, PGM, RIP, RSVP, UDP: Modernize packet parsing style
       EGP: Replace custom code with tok2str()
       EIGRP: Get the packet header fields right.
       UDP: Clean up address and port printing.

diff --git a/print-rip.c b/print-rip.c
index 0a2bef3e6874537e4cec17c57b9671c4d8d6da94..e9262e10c0f68a10835978725502624e992dca66 100644 (file)
--- a/print-rip.c
+++ b/print-rip.c
@@ -29,6 +29,7 @@
 
 #include "netdissect-stdinc.h"
 
+#define ND_LONGJMP_FROM_TCHECK
 #include "netdissect.h"
 #include "addrtoname.h"
 #include "extract.h"
@@ -195,7 +196,7 @@ rip_entry_print_v1(netdissect_options *ndo, const u_char *p,
 
        /* RFC 1058 */
        if (remaining < RIP_ROUTELEN)
-               return (0);
+               goto invalid;
        ND_TCHECK_SIZE(ni);
        family = GET_BE_U_2(ni->rip_family);
        if (family != BSD_AFNUM_INET && family != 0) {
@@ -220,7 +221,7 @@ rip_entry_print_v1(netdissect_options *ndo, const u_char *p,
                 GET_IPADDR_STRING(ni->rip_dest),
                 GET_BE_U_4(ni->rip_metric));
        return (RIP_ROUTELEN);
-trunc:
+invalid:
        return 0;
 }
 
@@ -233,7 +234,7 @@ rip_entry_print_v2(netdissect_options *ndo, const u_char *p,
        const struct rip_netinfo_v2 *ni;
 
        if (remaining < sizeof(*eh))
-               return (0);
+               goto invalid;
        ND_TCHECK_SIZE(eh);
        family = GET_BE_U_2(eh->rip_family);
        if (family == 0xFFFF) { /* variable-sized authentication structures */
@@ -248,9 +249,8 @@ rip_entry_print_v2(netdissect_options *ndo, const u_char *p,
                        const struct rip_auth_crypto_v2 *ch;
 
                        ch = (const struct rip_auth_crypto_v2 *)p;
-                       ND_TCHECK_SIZE(ch);
                        if (remaining < sizeof(*ch))
-                               return (0);
+                               goto invalid;
                        ND_PRINT("\n\t  Auth header:");
                        ND_PRINT(" Packet Len %u,",
                                 GET_BE_U_2(ch->rip_packet_len));
@@ -275,9 +275,8 @@ rip_entry_print_v2(netdissect_options *ndo, const u_char *p,
                print_unknown_data(ndo, p + sizeof(*eh), "\n\t  ", RIP_ROUTELEN - sizeof(*eh));
        } else { /* BSD_AFNUM_INET or AFI 0 */
                ni = (const struct rip_netinfo_v2 *)p;
-               ND_TCHECK_SIZE(ni);
                if (remaining < sizeof(*ni))
-                       return (0);
+                       goto invalid;
                ND_PRINT("\n\t  AFI %s, %15s/%-2d, tag 0x%04x, metric: %u, next-hop: ",
                         tok2str(bsd_af_values, "%u", family),
                         GET_IPADDR_STRING(ni->rip_dest),
@@ -290,37 +289,26 @@ rip_entry_print_v2(netdissect_options *ndo, const u_char *p,
                        ND_PRINT("self");
        }
        return (RIP_ROUTELEN);
-trunc:
+invalid:
        return 0;
 }
 
 void
 rip_print(netdissect_options *ndo,
-         const u_char *dat, u_int length)
+         const u_char *p, u_int len)
 {
        const struct rip *rp;
        uint8_t vers, cmd;
-       const u_char *p;
-       u_int len, routecount;
        unsigned entry_size;
 
        ndo->ndo_protocol = "rip";
-       if (ndo->ndo_snapend < dat) {
-               nd_print_trunc(ndo);
-               return;
-       }
-       len = ND_BYTES_AVAILABLE_AFTER(dat);
-       if (len > length)
-               len = length;
        if (len < sizeof(*rp)) {
-               nd_print_trunc(ndo);
-               return;
+               ND_PRINT(" (packet length %u)", len);
+               goto invalid;
        }
-       len -= sizeof(*rp);
 
-       rp = (const struct rip *)dat;
+       rp = (const struct rip *)p;
 
-       ND_TCHECK_SIZE(rp);
        vers = GET_U_1(rp->rip_vers);
        ND_PRINT("%sRIPv%u",
                 (ndo->ndo_vflag >= 1) ? "\n\t" : "",
@@ -330,10 +318,13 @@ rip_print(netdissect_options *ndo,
        cmd = GET_U_1(rp->rip_cmd);
        ND_PRINT(", %s, length: %u",
                tok2str(rip_cmd_values, "unknown command (%u)", cmd),
-               length);
+               len);
 
+       ND_TCHECK_SIZE(rp);
        if (ndo->ndo_vflag < 1)
                return;
+       p += sizeof(*rp);
+       len -= sizeof(*rp);
 
        switch (cmd) {
 
@@ -342,21 +333,17 @@ rip_print(netdissect_options *ndo,
                switch (vers) {
 
                case 1:
-                       routecount = length / RIP_ROUTELEN;
-                       ND_PRINT(", routes: %u", routecount);
-                       p = (const u_char *)(rp + 1);
+                       ND_PRINT(", routes: %u", len / RIP_ROUTELEN);
                        while (len != 0) {
                                entry_size = rip_entry_print_v1(ndo, p, len);
                                if (entry_size == 0) {
                                        /* Error */
-                                       nd_print_trunc(ndo);
-                                       break;
+                                       goto invalid;
                                }
                                if (len < entry_size) {
                                        ND_PRINT(" [remaining entries length %u < %u]",
                                                 len, entry_size);
-                                       nd_print_invalid(ndo);
-                                       break;
+                                       goto invalid;
                                }
                                p += entry_size;
                                len -= entry_size;
@@ -364,21 +351,17 @@ rip_print(netdissect_options *ndo,
                        break;
 
                case 2:
-                       routecount = length / RIP_ROUTELEN;
-                       ND_PRINT(", routes: %u or less", routecount);
-                       p = (const u_char *)(rp + 1);
+                       ND_PRINT(", routes: %u or less", len / RIP_ROUTELEN);
                        while (len != 0) {
                                entry_size = rip_entry_print_v2(ndo, p, len);
                                if (entry_size == 0) {
                                        /* Error */
-                                       nd_print_trunc(ndo);
-                                       break;
+                                       goto invalid;
                                }
                                if (len < entry_size) {
                                        ND_PRINT(" [remaining entries length %u < %u]",
                                                 len, entry_size);
-                                       nd_print_invalid(ndo);
-                                       break;
+                                       goto invalid;
                                }
                                p += entry_size;
                                len -= entry_size;
@@ -403,16 +386,18 @@ rip_print(netdissect_options *ndo,
 
        default:
                if (ndo->ndo_vflag <= 1) {
-                       if (!print_unknown_data(ndo, (const uint8_t *)rp, "\n\t", length))
+                       if (!print_unknown_data(ndo, p, "\n\t", len))
                                return;
                }
                break;
        }
        /* do we want to see an additionally hexdump ? */
        if (ndo->ndo_vflag> 1) {
-               if (!print_unknown_data(ndo, (const uint8_t *)rp, "\n\t", length))
+               if (!print_unknown_data(ndo, p, "\n\t", len))
                        return;
        }
-trunc:
        return;
+invalid:
+       nd_print_invalid(ndo);
+       ND_TCHECK_LEN(p, len);
 }

diff --git a/tests/ripv2-invalid-length.out b/tests/ripv2-invalid-length.out
index a0c9e3d06a74a39a6d64e8baff66162610455b85..8e04d1c6802e149abe868ed31d9daf86afda54a4 100644 (file)
--- a/tests/ripv2-invalid-length.out
+++ b/tests/ripv2-invalid-length.out
@@ -1,6 +1,6 @@
     1  08:36:15.227124 IP (tos 0xc0, ttl 2, id 0, offset 0, flags [none], proto UDP (17), length 192)
     10.7.56.254.520 > 224.0.0.9.520: 
-       RIPv2, Response, length: 160, routes: 8 or less
+       RIPv2, Response, length: 160, routes: 7 or less
          AFI IPv4,        10.7.0.0/24, tag 0x0000, metric: 1, next-hop: self
          AFI IPv4,       10.7.41.0/24, tag 0x0000, metric: 1, next-hop: self
          AFI IPv4,       10.7.51.0/24, tag 0x0000, metric: 1, next-hop: self

diff --git a/tests/ripv2_auth.out b/tests/ripv2_auth.out
index acdb1063ee3dac4db4e8845bc20c961e7e45f3f8..e332ec8d4455ede1e39bcc78037271d4dca33b67 100644 (file)
--- a/tests/ripv2_auth.out
+++ b/tests/ripv2_auth.out
@@ -40,7 +40,7 @@
          0x0010:  3375 fc89
     7  15:49:00.891527 IP (tos 0xc0, ttl 1, id 0, offset 0, flags [DF], proto UDP (17), length 108)
     10.0.0.20.520 > 224.0.0.9.520: 
-       RIPv2, Request, length: 80, routes: 4 or less
+       RIPv2, Request, length: 80, routes: 3 or less
          Auth header: Packet Len 44, Key-ID 45, Auth Data Len 32, SeqNo 1339429740, MBZ 0, MBZ 0
          AFI 0,         0.0.0.0/0 , tag 0x0000, metric: 16, next-hop: self
          Auth trailer:
@@ -48,7 +48,7 @@
          0x0010:  451a bd20 cee4 8a3d a466 17a0 e550 5b4b
     8  15:49:04.890122 IP (tos 0xc0, ttl 1, id 0, offset 0, flags [DF], proto UDP (17), length 108)
     10.0.0.20.520 > 224.0.0.9.520: 
-       RIPv2, Response, length: 80, routes: 4 or less
+       RIPv2, Response, length: 80, routes: 3 or less
          Auth header: Packet Len 44, Key-ID 45, Auth Data Len 32, SeqNo 1339429744, MBZ 0, MBZ 0
          AFI IPv4,     10.70.178.0/24, tag 0x0000, metric: 1, next-hop: self
          Auth trailer: