]> The Tcpdump Group git mirrors - tcpdump/commitdiff
Make sure we don't set the snapend before the beginning of the packet.
authorGuy Harris <[email protected]>
Tue, 29 Mar 2022 07:51:21 +0000 (00:51 -0700)
committerGuy Harris <[email protected]>
Tue, 29 Mar 2022 07:51:21 +0000 (00:51 -0700)
If a caller attempts to set it after the current snapend, just silently
ignore the attempt.

If they try to set it before the beginning of the packet, report it as a
bug and quit dissection.  Add a new setjmp() return value meaning "bug"
rather than "truncated packet", add an "nd_bug_longjmp()" routine for
reporting bugs where we should quit dissecting, and use that in this
case.

netdissect.c
netdissect.h
print.c

index 7e46d6aa3d3176ede396fefee8c788d7a053db03..a706a6a0845e9240cd79398d3245a49bddd141d8 100644 (file)
@@ -185,9 +185,31 @@ nd_push_snapend(netdissect_options *ndo, const u_char *new_snapend)
        ndspi->ndspi_snapend = ndo->ndo_snapend;
        ndspi->ndspi_prev = ndo->ndo_packet_info_stack;
 
-       /* No new packet pointer, either */
-       if (new_snapend < ndo->ndo_snapend)
-               ndo->ndo_snapend = new_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.
+        *
+        * 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.
+        */
+       if (new_snapend <= ndo->ndo_snapend) {
+               /* 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_packet_info_stack = ndspi;
 
        return (1);     /* success */
@@ -203,14 +225,37 @@ void
 nd_change_snapend(netdissect_options *ndo, const u_char *new_snapend)
 {
        struct netdissect_saved_packet_info *ndspi;
+       const u_char *previous_snapend;
 
        ndspi = ndo->ndo_packet_info_stack;
-       if (ndspi->ndspi_prev != NULL) {
-               if (new_snapend <= ndspi->ndspi_prev->ndspi_snapend)
-                       ndo->ndo_snapend = new_snapend;
-       } else {
-               if (new_snapend < ndo->ndo_snapend)
+       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.
+        *
+        * 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.
+        */
+       if (new_snapend <= previous_snapend) {
+               /* 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);
+               }
        }
 }
 
index e2b778601bc5d87010340d49bb91c0fb67203832..605a9c9bde7888562cae0e438755fb3115557ad4 100644 (file)
@@ -193,7 +193,8 @@ struct netdissect_saved_packet_info {
 };
 
 /* 'val' value(s) for longjmp */
-#define ND_TRUNCATED 1
+#define ND_TRUNCATED 1          /* packet data too short */
+#define ND_BUG       2          /* bug of some sort */
 
 struct netdissect_options {
   int ndo_bflag;               /* print 4 byte ASes in ASDOT notation */
@@ -283,6 +284,20 @@ 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 */
diff --git a/print.c b/print.c
index fbb1b3b1085116f10436a405f238e519b886d422..b595a882a7e630cf3f59b6f367d181955fe2ce0a 100644 (file)
--- a/print.c
+++ b/print.c
@@ -421,6 +421,13 @@ pretty_print_packet(netdissect_options *ndo, const struct pcap_pkthdr *h,
                profile_func_level = pretty_print_packet_level;
 #endif
                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;