Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 3 Sep 2024 17:26:28 GMT
From:      Joseph Mingrone <jrm@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: f8860353d4f4 - main - tcpdump: ppp: Use the buffer stack for the de-escaping buffer
Message-ID:  <202409031726.483HQS5l027174@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jrm:

URL: https://cgit.FreeBSD.org/src/commit/?id=f8860353d4f4c25bacdae5bc1cfb7a95edc9bfe0

commit f8860353d4f4c25bacdae5bc1cfb7a95edc9bfe0
Author:     Guy Harris <gharris@sonic.net>
AuthorDate: 2024-09-03 17:11:16 +0000
Commit:     Joseph Mingrone <jrm@FreeBSD.org>
CommitDate: 2024-09-03 17:24:16 +0000

    tcpdump: ppp: Use the buffer stack for the de-escaping buffer
    
    This both saves the buffer for freeing later and saves the packet
    pointer and snapend to be restored when packet processing is complete,
    even if an exception is thrown with longjmp.
    
    This means that the hex/ASCII printing in pretty_print_packet()
    processes the packet data as captured or read from the savefile, rather
    than as modified by the PPP printer, so that the bounds checking is
    correct.
    
    That fixes CVE-2024-2397, which was caused by an exception being thrown
    by the hex/ASCII printer (which should only happen if those routines are
    called by a packet printer, not if they're called for the -X/-x/-A
    flag), which jumps back to the setjmp() that surrounds the packet
    printer.  Hilarity^Winfinite looping ensues.
    
    Also, restore ndo->ndo_packetp before calling the hex/ASCII printing
    routine, in case nd_pop_all_packet_info() didn't restore it.
    
    Reviewed by:    emaste
---
 contrib/tcpdump/print-ppp.c | 31 +++++++++++++++++--------------
 contrib/tcpdump/print.c     |  8 ++++++--
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/tcpdump/print-ppp.c b/contrib/tcpdump/print-ppp.c
index aba243ddb6f2..e5ae0646ebae 100644
--- a/contrib/tcpdump/print-ppp.c
+++ b/contrib/tcpdump/print-ppp.c
@@ -42,6 +42,8 @@
 #include <net/if_ppp.h>
 #endif
 
+#include <stdlib.h>
+
 #include "netdissect.h"
 #include "extract.h"
 #include "addrtoname.h"
@@ -1363,7 +1365,6 @@ ppp_hdlc(netdissect_options *ndo,
 	u_char *b, *t, c;
 	const u_char *s;
 	u_int i, proto;
-	const void *sb, *se;
 
 	if (caplen == 0)
 		return;
@@ -1371,9 +1372,11 @@ ppp_hdlc(netdissect_options *ndo,
         if (length == 0)
                 return;
 
-	b = (u_char *)nd_malloc(ndo, caplen);
-	if (b == NULL)
-		return;
+	b = (u_char *)malloc(caplen);
+	if (b == NULL) {
+		(*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
+			"%s: malloc", __func__);
+	}
 
 	/*
 	 * Unescape all the data into a temporary, private, buffer.
@@ -1394,13 +1397,15 @@ ppp_hdlc(netdissect_options *ndo,
 	}
 
 	/*
-	 * Change the end pointer, so bounds checks work.
-	 * Change the pointer to packet data to help debugging.
+	 * Switch to the output buffer for dissection, and save it
+	 * on the buffer stack so it can be freed; our caller must
+	 * pop it when done.
 	 */
-	sb = ndo->ndo_packetp;
-	se = ndo->ndo_snapend;
-	ndo->ndo_packetp = b;
-	ndo->ndo_snapend = t;
+	if (!nd_push_buffer(ndo, b, b, (u_int)(t - b))) {
+		free(b);
+		(*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
+			"%s: can't push buffer on buffer stack", __func__);
+	}
 	length = ND_BYTES_AVAILABLE_AFTER(b);
 
         /* now lets guess about the payload codepoint format */
@@ -1442,13 +1447,11 @@ ppp_hdlc(netdissect_options *ndo,
         }
 
 cleanup:
-	ndo->ndo_packetp = sb;
-	ndo->ndo_snapend = se;
+	nd_pop_packet_info(ndo);
         return;
 
 trunc:
-	ndo->ndo_packetp = sb;
-	ndo->ndo_snapend = se;
+	nd_pop_packet_info(ndo);
 	nd_print_trunc(ndo);
 }
 
diff --git a/contrib/tcpdump/print.c b/contrib/tcpdump/print.c
index 41a6b524fbf8..96d34b772f08 100644
--- a/contrib/tcpdump/print.c
+++ b/contrib/tcpdump/print.c
@@ -434,10 +434,14 @@ pretty_print_packet(netdissect_options *ndo, const struct pcap_pkthdr *h,
 	nd_pop_all_packet_info(ndo);
 
 	/*
-	 * Restore the original snapend, as a printer might have
-	 * changed it.
+	 * Restore the originals snapend and packetp, as a printer
+	 * might have changed them.
+	 *
+	 * XXX - nd_pop_all_packet_info() should have restored the
+	 * original values, but, just in case....
 	 */
 	ndo->ndo_snapend = sp + h->caplen;
+	ndo->ndo_packetp = sp;
 	if (ndo->ndo_Xflag) {
 		/*
 		 * Print the raw packet data in hex and ASCII.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202409031726.483HQS5l027174>