]> The Tcpdump Group git mirrors - tcpdump/commitdiff
Have routines that set the snapend take a buffer pointer and length as args.
authorGuy Harris <[email protected]>
Thu, 31 Mar 2022 09:29:19 +0000 (02:29 -0700)
committerGuy Harris <[email protected]>
Sat, 2 Apr 2022 00:22:55 +0000 (17:22 -0700)
Have nd_push_buffer() take a snapshot length, not a snapshot end, as
its last argument.

Replace nd_push_snapend() and nd_change_snapend() with nd_push_snaplen()
and nd_change_snaplen(), both of which take a pointer into the packet
buffer and snapshot length relative to that pointer as arguments.  Have
those routines check the snapshot length to make sure it's not bigger
than the number of bytes in the packet past the pointer, and silently
ignore the requst if it is.

Using a length rather than a pointer avoids the possibility of the
calculation of the snapshot end overflowing and resulting in a snapshot
end *before* the point in the buffer.

Add a test for this, with a capture file containing an IPv6 packet with
an extremely large "jumbo" packet size.

Revert the "Make sure we don't set the snapend before the beginning of
the packet." changes, as they no longer apply with this change (which
also makes sure we don't set the snapend before the beginning of the
packet).

(cherry picked from commit 6a681e6a16943fb363b5403e84272a1ddaccf28e)

netdissect.c
netdissect.h
print-esp.c
print-ether.c
print-ip.c
print-ip6.c
print.c
tests/TESTLIST
tests/ipv6-too-long-jumbo.out [new file with mode: 0644]
tests/ipv6-too-long-jumbo.pcap [new file with mode: 0644]

index a706a6a0845e9240cd79398d3245a49bddd141d8..72923a174297ca4f8285720eb70e0dbf6a398c5d 100644 (file)
@@ -149,7 +149,7 @@ nd_smi_version_string(void)
 
 int
 nd_push_buffer(netdissect_options *ndo, u_char *new_buffer,
-    const u_char *new_packetp, const u_char *new_snapend)
+    const u_char *new_packetp, u_int newlen)
 {
        struct netdissect_saved_packet_info *ndspi;
 
@@ -162,20 +162,30 @@ nd_push_buffer(netdissect_options *ndo, u_char *new_buffer,
        ndspi->ndspi_prev = ndo->ndo_packet_info_stack;
 
        ndo->ndo_packetp = new_packetp;
-       ndo->ndo_snapend = new_snapend;
+       ndo->ndo_snapend = new_packetp + newlen;
        ndo->ndo_packet_info_stack = ndspi;
 
        return (1);     /* success */
 }
 
+
 /*
- * Set a new snapshot end to the minimum of the existing snapshot end
- * and the new snapshot end.
+ * In a given netdissect_options structure:
+ *
+ *,   push the current packet information onto the packet informaton
+ *    stack;
+ *
+ *    given a pointer into the packet and a length past that point in
+ *    the packet, calculate a new snapshot end that's at the lower
+ *    of the current snapshot end and that point in the packet;
+ *
+ *    set the snapshot end to that new value.
  */
 int
-nd_push_snapend(netdissect_options *ndo, const u_char *new_snapend)
+nd_push_snaplen(netdissect_options *ndo, const u_char *bp, u_int newlen)
 {
        struct netdissect_saved_packet_info *ndspi;
+       u_int snaplen_remaining;
 
        ndspi = (struct netdissect_saved_packet_info *)malloc(sizeof(struct netdissect_saved_packet_info));
        if (ndspi == NULL)
@@ -186,76 +196,86 @@ nd_push_snapend(netdissect_options *ndo, const u_char *new_snapend)
        ndspi->ndspi_prev = ndo->ndo_packet_info_stack;
 
        /*
-        * Make sure the new snapend is sane.
-        *
-        * If it's after the current snapend, it's not valid.  We
-        * silently ignore the new setting; that means that our callers
-        * don't have to do this check themselves, and also means that
-        * if the new length is used when dissecting, we'll go past the
-        * snapend and report an error.
+        * Push the saved previous data onto the stack.
+        */
+       ndo->ndo_packet_info_stack = ndspi;
+
+       /*
+        * Find out how many bytes remain after the current snapend.
         *
-        * If it's before the beginning of the packet, it's not valid.
-        * That "should not happen", but might happen with a *very*
-        * large adjustment to the snapend; our callers *should* check
-        * for that, so we fail if they haven't done so.
+        * We're restricted to packets with at most UINT_MAX bytes;
+        * cast the result to u_int, so that we don't get truncation
+        * warnings on LP64 and LLP64 platforms.  (ptrdiff_t is
+        * signed and we want an unsigned difference; the pointer
+        * should at most be equal to snapend, and must *never*
+        * be past snapend.)
         */
-       if (new_snapend <= ndo->ndo_snapend) {
+       snaplen_remaining = (u_int)(ndo->ndo_snapend - bp);
+
+       /*
+        * If the new snapend is smaller than the one calculated
+        * above, set the snapend to that value, otherwise leave
+        * it unchanged.
+        */
+       if (newlen <= snaplen_remaining) {
                /* Snapend isn't past the previous snapend */
-               if (new_snapend >= ndo->ndo_packetp) {
-                       /* And it isn't before the beginning of the packet */
-                       ndo->ndo_snapend = new_snapend;
-               } else {
-                       /* But it's before the beginning of the packet */
-                       ND_PRINT(" [new snapend before beginning of packet in nd_push_snapend]");
-                       nd_bug_longjmp(ndo);
-               }
+               ndo->ndo_snapend = bp + newlen;
        }
-       ndo->ndo_packet_info_stack = ndspi;
 
        return (1);     /* success */
 }
 
 /*
- * Change an already-pushed snapshot end.  This may increase the
+ * In a given netdissect_options structure:
+ *
+ *    given a pointer into the packet and a length past that point in
+ *    the packet, calculate a new snapshot end that's at the lower
+ *    of the previous snapshot end - or, if there is no previous
+ *    snapshot end, the current snapshot end - and that point in the
+ *    packet;
+ *
+ *    set the snapshot end to that new value.
+ *
+ * This is to change the current snapshot end.  This may increase the
  * snapshot end, as it may be used, for example, for a Jumbo Payload
  * option in IPv6.  It must not increase it past the snapshot length
  * atop which the current one was pushed, however.
  */
 void
-nd_change_snapend(netdissect_options *ndo, const u_char *new_snapend)
+nd_change_snaplen(netdissect_options *ndo, const u_char *bp, u_int newlen)
 {
        struct netdissect_saved_packet_info *ndspi;
        const u_char *previous_snapend;
+       u_int snaplen_remaining;
 
        ndspi = ndo->ndo_packet_info_stack;
        if (ndspi->ndspi_prev != NULL)
                previous_snapend = ndspi->ndspi_prev->ndspi_snapend;
        else
                previous_snapend = ndo->ndo_snapend;
+
        /*
-        * Make sure the new snapend is sane.
-        *
-        * If it's after the current snapend, it's not valid.  We
-        * silently ignore the new setting; that means that our callers
-        * don't have to do this check themselves, and also means that
-        * if the new length is used when dissecting, we'll go past the
-        * snapend and report an error.
+        * Find out how many bytes remain after the previous
+        * snapend - or, if there is no previous snapend, after
+        * the current snapend.
         *
-        * If it's before the beginning of the packet, it's not valid.
-        * That "should not happen", but might happen with a *very*
-        * large adjustment to the snapend; our callers *should* check
-        * for that, so we fail if they haven't done so.
+        * We're restricted to packets with at most UINT_MAX bytes;
+        * cast the result to u_int, so that we don't get truncation
+        * warnings on LP64 and LLP64 platforms.  (ptrdiff_t is
+        * signed and we want an unsigned difference; the pointer
+        * should at most be equal to snapend, and must *never*
+        * be past snapend.)
+        */
+       snaplen_remaining = (u_int)(previous_snapend - bp);
+
+       /*
+        * If the new snapend is smaller than the one calculated
+        * above, set the snapend to that value, otherwise leave
+        * it unchanged.
         */
-       if (new_snapend <= previous_snapend) {
+       if (newlen <= snaplen_remaining) {
                /* Snapend isn't past the previous snapend */
-               if (new_snapend >= ndo->ndo_packetp) {
-                       /* And it isn't before the beginning of the packet */
-                       ndo->ndo_snapend = new_snapend;
-               } else {
-                       /* But it's before the beginning of the packet */
-                       ND_PRINT(" [new snapend before beginning of packet in nd_push_snapend]");
-                       nd_bug_longjmp(ndo);
-               }
+               ndo->ndo_snapend = bp + newlen;
        }
 }
 
index 1c999a3ef9f72b861b97eb48b6ce6058f24b15e6..8aae13f6219418a46c163fd3d684e93db4865a83 100644 (file)
@@ -192,8 +192,7 @@ struct netdissect_saved_packet_info {
 };
 
 /* 'val' value(s) for longjmp */
-#define ND_TRUNCATED 1          /* packet data too short */
-#define ND_BUG       2          /* bug of some sort */
+#define ND_TRUNCATED 1
 
 struct netdissect_options {
   int ndo_bflag;               /* print 4 byte ASes in ASDOT notation */
@@ -262,9 +261,9 @@ struct netdissect_options {
 };
 
 extern int nd_push_buffer(netdissect_options *, u_char *, const u_char *,
-    const u_char *);
-extern int nd_push_snapend(netdissect_options *, const u_char *);
-extern void nd_change_snapend(netdissect_options *, const u_char *);
+    u_int);
+extern int nd_push_snaplen(netdissect_options *, const u_char *, u_int);
+extern void nd_change_snaplen(netdissect_options *, const u_char *, u_int);
 extern void nd_pop_packet_info(netdissect_options *);
 extern void nd_pop_all_packet_info(netdissect_options *);
 
@@ -282,20 +281,6 @@ nd_trunc_longjmp(netdissect_options *ndo)
 #endif /* _AIX */
 }
 
-static inline NORETURN void
-nd_bug_longjmp(netdissect_options *ndo)
-{
-       longjmp(ndo->ndo_early_end, ND_BUG);
-#ifdef _AIX
-       /*
-        * In AIX <setjmp.h> decorates longjmp() with "#pragma leaves", which tells
-        * XL C that the function is noreturn, but GCC remains unaware of that and
-        * yields a "'noreturn' function does return" warning.
-        */
-       ND_UNREACHABLE
-#endif /* _AIX */
-}
-
 #define PT_VAT         1       /* Visual Audio Tool */
 #define PT_WB          2       /* distributed White Board */
 #define PT_RPC         3       /* Remote Procedure Call */
index 4850d4a14c13c39b035baafe75cc233e9fba279d..31f93db09b34d1b05d4342c1417552a86bd1d5cb 100644 (file)
@@ -329,7 +329,7 @@ int esp_decrypt_buffer_by_ikev2_print(netdissect_options *ndo,
         * on the buffer stack so it can be freed; our caller must
         * pop it when done.
         */
-       if (!nd_push_buffer(ndo, pt, pt, pt + ctlen)) {
+       if (!nd_push_buffer(ndo, pt, pt, ctlen)) {
                free(pt);
                return 0;
        }
@@ -877,8 +877,7 @@ esp_print(netdissect_options *ndo,
         * Switch to the output buffer for dissection, and
         * save it on the buffer stack so it can be freed.
         */
-       ep = pt + payloadlen;
-       if (!nd_push_buffer(ndo, pt, pt, ep)) {
+       if (!nd_push_buffer(ndo, pt, pt, payloadlen)) {
                free(pt);
                (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
                        "%s: can't push buffer on buffer stack", __func__);
@@ -893,14 +892,14 @@ esp_print(netdissect_options *ndo,
         * it was not decrypted with the correct key, so that the
         * "plaintext" is not what was being sent.
         */
-       padlen = GET_U_1(ep - 2);
+       padlen = GET_U_1(pt + payloadlen - 2);
        if (padlen + 2 > payloadlen) {
                nd_print_trunc(ndo);
                return;
        }
 
        /* Get the next header */
-       nh = GET_U_1(ep - 1);
+       nh = GET_U_1(pt + payloadlen - 1);
 
        ND_PRINT(": ");
 
@@ -908,7 +907,7 @@ esp_print(netdissect_options *ndo,
         * Don't put padding + padding length(1 byte) + next header(1 byte)
         * in the buffer because they are not part of the plaintext to decode.
         */
-       nd_push_snapend(ndo, ep - (padlen + 2));
+       nd_push_snaplen(ndo, pt, payloadlen - (padlen + 2));
 
        /* Now dissect the plaintext. */
        ip_demux_print(ndo, pt, payloadlen - (padlen + 2), ver, fragmented,
@@ -916,7 +915,7 @@ esp_print(netdissect_options *ndo,
 
        /* Pop the buffer, freeing it. */
        nd_pop_packet_info(ndo);
-       /* Pop the nd_push_snapend */
+       /* Pop the nd_push_snaplen */
        nd_pop_packet_info(ndo);
 #endif
 }
index 976ab4b696f56a2d30847f8d152dae27d5071057..2f2b150316a67f8e442857bb9e5d768f114f3a47 100644 (file)
@@ -306,7 +306,7 @@ recurse:
                 * Cut off the snapshot length to the end of the
                 * payload.
                 */
-               nd_push_snapend(ndo, p + length);
+               nd_push_snaplen(ndo, p, length);
 
                if (ndo->ndo_eflag) {
                        ND_PRINT("802.3");
index a0df95918898ff3228e760d0c07942790d3e3074..4f9617a3c6a8b205225ffffda44131c5c3afef04 100644 (file)
@@ -377,7 +377,7 @@ ip_print(netdissect_options *ndo,
        /*
         * Cut off the snapshot length to the end of the IP payload.
         */
-       nd_push_snapend(ndo, bp + len);
+       nd_push_snaplen(ndo, bp, len);
 
        len -= hlen;
 
index b3379baaeb7ced31cd3df9b6938b0a3a73ca64cb..15b30630fb2418330246f3c6770439b540cce168 100644 (file)
@@ -305,7 +305,7 @@ ip6_print(netdissect_options *ndo, const u_char *bp, u_int length)
        /*
         * Cut off the snapshot length to the end of the IP payload.
         */
-       nd_push_snapend(ndo, bp + len);
+       nd_push_snaplen(ndo, bp, len);
 
        cp = (const u_char *)ip6;
        advance = sizeof(struct ip6_hdr);
@@ -413,7 +413,7 @@ ip6_print(netdissect_options *ndo, const u_char *bp, u_int length)
                                if (length < len)
                                        ND_PRINT("truncated-ip6 - %u bytes missing!",
                                                len - length);
-                               nd_change_snapend(ndo, bp + len);
+                               nd_change_snaplen(ndo, bp, len);
 
                                /*
                                 * Now subtract the length of the IPv6
@@ -446,7 +446,7 @@ ip6_print(netdissect_options *ndo, const u_char *bp, u_int length)
                                         * accordingly.
                                         */
                                        len = sizeof(struct ip6_hdr);
-                                       nd_change_snapend(ndo, bp + len);
+                                       nd_change_snaplen(ndo, bp, len);
 
                                        /*
                                         * Now subtract the length of
diff --git a/print.c b/print.c
index 5f0a035289678b45c0fe7eae2ee78c7b5ed2ecd7..84aae66c95dfad4e91622038327db974b8e6539f 100644 (file)
--- a/print.c
+++ b/print.c
@@ -418,13 +418,6 @@ pretty_print_packet(netdissect_options *ndo, const struct pcap_pkthdr *h,
                /* Print the full packet */
                ndo->ndo_ll_hdr_len = 0;
                break;
-       case ND_BUG:
-               /*
-                * A printer or helper routine quit because a bug was
-                * detected; report it.
-                */
-               ND_PRINT(" [Bug in %s protocol printer]", ndo->ndo_protocol);
-               break;
        }
        hdrlen = ndo->ndo_ll_hdr_len;
 
index 24de5a9de3bf978974f6e7ad1541b8d69f844f1d..7020a9f3c6d85a5cc823fb793f49fa21595a798b 100644 (file)
@@ -299,6 +299,8 @@ ipv6-srh-ext-header ipv6-srh-ext-header.pcap        ipv6-srh-ext-header.out -v
 ipv6-srh-insert-cksum  ipv6-srh-insert-cksum.pcap      ipv6-srh-insert-cksum.out -v
 ipv6-srh-ipproto-ether-v ipv6-srh-ipproto-ether.pcap ipv6-srh-ipproto-ether-v.out -v
 ipv6-srh-ipproto-ether-ev ipv6-srh-ipproto-ether.pcap ipv6-srh-ipproto-ether-ev.out -ev
+ipv6-too-long-jumbo    ipv6-too-long-jumbo.pcap        ipv6-too-long-jumbo.out -v
+
 # Loopback/CTP test case
 loopback       loopback.pcap           loopback.out
 
diff --git a/tests/ipv6-too-long-jumbo.out b/tests/ipv6-too-long-jumbo.out
new file mode 100644 (file)
index 0000000..c5ccb83
--- /dev/null
@@ -0,0 +1 @@
+    1  12:40:23.226395 IP6 (class 0xc0, hlim 0, next-header Options (0) payload length: 0) 1:6:1a28:312:d7cb:b318:34e5:d3ea > 2b7f:cd1f:ec3c:fb9c:e731:d16b:a8fe:ba8c: HBH (opt_type 0x1a: len=0)(padn)(opt_type 0x16: len=0)(opt_type 0x64: len=114)(jumbo: 3858694210) (opt_type 0x42: len=3)(opt_type 0xfe: len=6)(pad1)(jumbo: 248 - already seen) (opt_type 0x0e: len=8)(opt_type 0x07: len=4)(opt_type 0xf1: len=60) truncated-ip6 - 3858693774 bytes missing! ip-proto-12 3858693802
diff --git a/tests/ipv6-too-long-jumbo.pcap b/tests/ipv6-too-long-jumbo.pcap
new file mode 100644 (file)
index 0000000..3a7fea9
Binary files /dev/null and b/tests/ipv6-too-long-jumbo.pcap differ