]> The Tcpdump Group git mirrors - tcpdump/commitdiff
projects / tcpdump / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: b5cbb4d)
DTP: Modernize packet parsing style.
authorDenis Ovsienko <[email protected]>
Tue, 1 Dec 2020 00:05:32 +0000 (00:05 +0000)
committerDenis Ovsienko <[email protected]>
Tue, 1 Dec 2020 00:05:32 +0000 (00:05 +0000)
Enable ND_LONGJMP_FROM_TCHECK. Report an invalid packet as invalid with
a reason, not truncated. Test whether the invalid packet is entirely in
the buffer. Remove two redundant ND_TCHECK*() instances and make another
one conditional. Let nd_printzp() guard the packet buffer end.

Add more length checks and make existing ones stricter. Decrement the
remaining length instead of comparing pointers. Lose a duplicate "TLV"
in the output. Reduce scope of two variables and lose another one.
Update two tests.

print-dtp.c patch | blob | history
tests/dtp-v.out patch | blob | history
tests/rpvst-v.out patch | blob | history

diff --git a/print-dtp.c b/print-dtp.c
index 4c3d6c780eac1d1fa9a2e5de730ff1825425b2c3..a7cb9409227e515e5c20eacce60e9df451a0d391 100644 (file)
--- a/print-dtp.c
+++ b/print-dtp.c
@@ -23,6 +23,7 @@
 
 #include "netdissect-stdinc.h"
 
+#define ND_LONGJMP_FROM_TCHECK
 #include "netdissect.h"
 #include "addrtoname.h"
 #include "extract.h"
@@ -35,26 +36,21 @@
 #define DTP_NEIGHBOR_TLV               0x0004
 
 static const struct tok dtp_tlv_values[] = {
-    { DTP_DOMAIN_TLV, "Domain TLV"},
-    { DTP_STATUS_TLV, "Status TLV"},
-    { DTP_DTP_TYPE_TLV, "DTP type TLV"},
-    { DTP_NEIGHBOR_TLV, "Neighbor TLV"},
+    { DTP_DOMAIN_TLV, "Domain" },
+    { DTP_STATUS_TLV, "Status" },
+    { DTP_DTP_TYPE_TLV, "DTP type" },
+    { DTP_NEIGHBOR_TLV, "Neighbor" },
     { 0, NULL}
 };
 
 void
-dtp_print(netdissect_options *ndo, const u_char *pptr, u_int length)
+dtp_print(netdissect_options *ndo, const u_char *tptr, u_int length)
 {
-    int type, len;
-    const u_char *tptr;
-
     ndo->ndo_protocol = "dtp";
-    if (length < DTP_HEADER_LEN)
-        goto trunc;
-
-    tptr = pptr;
-
-    ND_TCHECK_LEN(tptr, DTP_HEADER_LEN);
+    if (length < DTP_HEADER_LEN) {
+        ND_PRINT("[zero packet length]");
+        goto invalid;
+    }
 
     ND_PRINT("DTPv%u, length %u",
            GET_U_1(tptr),
@@ -68,10 +64,15 @@ dtp_print(netdissect_options *ndo, const u_char *pptr, u_int length)
     }
 
     tptr += DTP_HEADER_LEN;
+    length -= DTP_HEADER_LEN;
 
-    while (tptr < (pptr+length)) {
+    while (length) {
+        uint16_t type, len;
 
-        ND_TCHECK_4(tptr);
+        if (length < 4) {
+            ND_PRINT("[%u bytes remaining]", length);
+            goto invalid;
+        }
        type = GET_BE_U_2(tptr);
         len  = GET_BE_U_2(tptr + 2);
        /* XXX: should not be but sometimes it is, see the test captures */
@@ -82,40 +83,40 @@ dtp_print(netdissect_options *ndo, const u_char *pptr, u_int length)
                type, len);
 
         /* infinite loop check */
-        if (len < 4)
+        if (len < 4 || len > length) {
+            ND_PRINT("[invalid TLV length %u]", len);
             goto invalid;
-        ND_TCHECK_LEN(tptr, len);
+        }
 
         switch (type) {
        case DTP_DOMAIN_TLV:
                ND_PRINT(", ");
-               nd_printzp(ndo, tptr+4, len-4, pptr+length);
+               nd_printzp(ndo, tptr+4, len-4, NULL);
                break;
 
        case DTP_STATUS_TLV:
        case DTP_DTP_TYPE_TLV:
-                if (len < 5)
+                if (len != 5)
                     goto invalid;
                 ND_PRINT(", 0x%x", GET_U_1(tptr + 4));
                 break;
 
        case DTP_NEIGHBOR_TLV:
-                if (len < 10)
+                if (len != 10)
                     goto invalid;
                 ND_PRINT(", %s", GET_ETHERADDR_STRING(tptr+4));
                 break;
 
         default:
+            ND_TCHECK_LEN(tptr, len);
             break;
         }
         tptr += len;
+        length -= len;
     }
-
     return;
 
  invalid:
     nd_print_invalid(ndo);
-    return;
- trunc:
-    nd_print_trunc(ndo);
+    ND_TCHECK_LEN(tptr, length);
 }
diff --git a/tests/dtp-v.out b/tests/dtp-v.out
index b31d4567b7fe769c56e4cdcf6435500155e74493..7987a323fdef10432fb004afce71da9d83ec61a9 100644 (file)
--- a/tests/dtp-v.out
+++ b/tests/dtp-v.out
@@ -1,8 +1,8 @@
     1  11:46:11.550072 DTPv1, length 29
-       Domain TLV (0x0001) TLV, length 8, Lab
-       Status TLV (0x0002) TLV, length 5, 0x4
-       DTP type TLV (0x0003) TLV, length 5, 0x40
-       Neighbor TLV (0x0004) TLV, length 10, 00:19:06:ea:b8:85
+       Domain (0x0001) TLV, length 8, Lab
+       Status (0x0002) TLV, length 5, 0x4
+       DTP type (0x0003) TLV, length 5, 0x40
+       Neighbor (0x0004) TLV, length 10, 00:19:06:ea:b8:85
     2  11:46:11.550157 00:19:06:ea:b8:85 > 01:00:0c:00:00:00 SNAP, oui Cisco (0x00000c), pid Unknown (0x0003), length 68: 
        0x0000:  aaaa 0300 000c 0003 0000 0000 0100 0ccc  ................
        0x0010:  cccc 0019 06ea b885 0025 aaaa 0300 000c  .........%......
@@ -10,10 +10,10 @@
        0x0030:  0003 0005 4000 0400 0a00 1906 eab8 8500  ....@...........
        0x0040:  0000 0000 0000 0000 f7a7 fe42            ...........B
     3  11:46:41.559806 DTPv1, length 29
-       Domain TLV (0x0001) TLV, length 8, Lab
-       Status TLV (0x0002) TLV, length 5, 0x4
-       DTP type TLV (0x0003) TLV, length 5, 0x40
-       Neighbor TLV (0x0004) TLV, length 10, 00:19:06:ea:b8:85
+       Domain (0x0001) TLV, length 8, Lab
+       Status (0x0002) TLV, length 5, 0x4
+       DTP type (0x0003) TLV, length 5, 0x40
+       Neighbor (0x0004) TLV, length 10, 00:19:06:ea:b8:85
     4  11:46:41.559890 00:19:06:ea:b8:85 > 01:00:0c:00:00:00 SNAP, oui Cisco (0x00000c), pid Unknown (0x0003), length 68: 
        0x0000:  aaaa 0300 000c 0003 0000 0000 0100 0ccc  ................
        0x0010:  cccc 0019 06ea b885 0025 aaaa 0300 000c  .........%......
@@ -21,10 +21,10 @@
        0x0030:  0003 0005 4000 0400 0a00 1906 eab8 8500  ....@...........
        0x0040:  0000 0000 0000 0000 f7a7 fe42            ...........B
     5  11:47:11.566606 DTPv1, length 29
-       Domain TLV (0x0001) TLV, length 8, Lab
-       Status TLV (0x0002) TLV, length 5, 0x4
-       DTP type TLV (0x0003) TLV, length 5, 0x40
-       Neighbor TLV (0x0004) TLV, length 10, 00:19:06:ea:b8:85
+       Domain (0x0001) TLV, length 8, Lab
+       Status (0x0002) TLV, length 5, 0x4
+       DTP type (0x0003) TLV, length 5, 0x40
+       Neighbor (0x0004) TLV, length 10, 00:19:06:ea:b8:85
     6  11:47:11.566691 00:19:06:ea:b8:85 > 01:00:0c:00:00:00 SNAP, oui Cisco (0x00000c), pid Unknown (0x0003), length 68: 
        0x0000:  aaaa 0300 000c 0003 0000 0000 0100 0ccc  ................
        0x0010:  cccc 0019 06ea b885 0025 aaaa 0300 000c  .........%......
@@ -32,10 +32,10 @@
        0x0030:  0003 0005 4000 0400 0a00 1906 eab8 8500  ....@...........
        0x0040:  0000 0000 0000 0000 f7a7 fe42            ...........B
     7  11:47:41.576275 DTPv1, length 29
-       Domain TLV (0x0001) TLV, length 8, Lab
-       Status TLV (0x0002) TLV, length 5, 0x4
-       DTP type TLV (0x0003) TLV, length 5, 0x40
-       Neighbor TLV (0x0004) TLV, length 10, 00:19:06:ea:b8:85
+       Domain (0x0001) TLV, length 8, Lab
+       Status (0x0002) TLV, length 5, 0x4
+       DTP type (0x0003) TLV, length 5, 0x40
+       Neighbor (0x0004) TLV, length 10, 00:19:06:ea:b8:85
     8  11:47:41.576360 00:19:06:ea:b8:85 > 01:00:0c:00:00:00 SNAP, oui Cisco (0x00000c), pid Unknown (0x0003), length 68: 
        0x0000:  aaaa 0300 000c 0003 0000 0000 0100 0ccc  ................
        0x0010:  cccc 0019 06ea b885 0025 aaaa 0300 000c  .........%......
@@ -43,10 +43,10 @@
        0x0030:  0003 0005 4000 0400 0a00 1906 eab8 8514  ....@...........
        0x0040:  0002 000f 0000 0000 7232 1da6            ........r2..
     9  11:48:11.585963 DTPv1, length 29
-       Domain TLV (0x0001) TLV, length 8, Lab
-       Status TLV (0x0002) TLV, length 5, 0x4
-       DTP type TLV (0x0003) TLV, length 5, 0x40
-       Neighbor TLV (0x0004) TLV, length 10, 00:19:06:ea:b8:85
+       Domain (0x0001) TLV, length 8, Lab
+       Status (0x0002) TLV, length 5, 0x4
+       DTP type (0x0003) TLV, length 5, 0x40
+       Neighbor (0x0004) TLV, length 10, 00:19:06:ea:b8:85
    10  11:48:11.586048 00:19:06:ea:b8:85 > 01:00:0c:00:00:00 SNAP, oui Cisco (0x00000c), pid Unknown (0x0003), length 68: 
        0x0000:  aaaa 0300 000c 0003 0000 0000 0100 0ccc  ................
        0x0010:  cccc 0019 06ea b885 0025 aaaa 0300 000c  .........%......
diff --git a/tests/rpvst-v.out b/tests/rpvst-v.out
index d2957112983ad4ab2b3ccc5114a1926b29c67c00..fbd2a2eecdedeea5e1a2557309e12a807950fca3 100644 (file)
--- a/tests/rpvst-v.out
+++ b/tests/rpvst-v.out
@@ -1,13 +1,13 @@
     1  10:39:19.323246 DTPv1, length 31
-       Domain TLV (0x0001) TLV, length 10, cisco
-       Status TLV (0x0002) TLV, length 5, 0x81
-       DTP type TLV (0x0003) TLV, length 5, 0xa5
-       Neighbor TLV (0x0004) TLV, length 10, 00:1f:6d:96:ec:04
+       Domain (0x0001) TLV, length 10, cisco
+       Status (0x0002) TLV, length 5, 0x81
+       DTP type (0x0003) TLV, length 5, 0xa5
+       Neighbor (0x0004) TLV, length 10, 00:1f:6d:96:ec:04
     2  10:39:20.329871 DTPv1, length 31
-       Domain TLV (0x0001) TLV, length 10, cisco
-       Status TLV (0x0002) TLV, length 5, 0x81
-       DTP type TLV (0x0003) TLV, length 5, 0xa5
-       Neighbor TLV (0x0004) TLV, length 10, 00:1f:6d:96:ec:04
+       Domain (0x0001) TLV, length 10, cisco
+       Status (0x0002) TLV, length 5, 0x81
+       DTP type (0x0003) TLV, length 5, 0xa5
+       Neighbor (0x0004) TLV, length 10, 00:1f:6d:96:ec:04
     3  10:39:21.327398 STP 802.1w, Rapid STP, Flags [Proposal], bridge-id 8001.00:1f:6d:96:ec:00.8004, length 42
        message-age 0.00s, max-age 20.00s, hello-time 2.00s, forwarding-delay 15.00s
        root-id 8001.00:1f:6d:96:ec:00, root-pathcost 0, port-role Designated
[mirror] the TCPdump network dissector