CVE-2024-2397: Insufficient Information (NVD-CWE-noinfo)
value: Due to a bug in packet data buffers management, the PPP printer in tcpdump can enter an infinite loop when reading a crafted DLT_PPP_SERIAL .pcap savefile.  This problem does not affect any tcpdump release, but it affected the git master branch from 2023-06-05 to 2024-03-21.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-04-12T14:15Z
Repository: https://github.com/the-tcpdump-group/tcpdump
Language: C
Files changed: print-ppp.c, print.c
Commit date: 2024-03-21 12:12:30 -0700
Commit message: 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.

Diff: diff --git a/print-ppp.c b/print-ppp.c
index 2cf06c3634..9aed23eb9a 100644
--- a/print-ppp.c
+++ b/print-ppp.c
@@ -35,10 +35,12 @@
 #include <config.h>
 #endif
 
 #include "netdissect-stdinc.h"
 
+#include <stdlib.h>
+
 #include "netdissect.h"
 #include "extract.h"
 #include "addrtoname.h"
 #include "ppp.h"
 #include "chdlc.h"
@@ -1356,21 +1358,22 @@ ppp_hdlc(netdissect_options *ndo,
 {
 	u_int caplen = ND_BYTES_AVAILABLE_AFTER(p);
 	u_char *b, *t, c;
 	const u_char *s;
 	u_int i, proto;
-	const void *sb, *se;
 
 	if (caplen == 0)
 		return;
 
         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.
 	 * Do this so that we don't overwrite the original packet
 	 * contents.
@@ -1387,17 +1390,19 @@ ppp_hdlc(netdissect_options *ndo,
 		}
 		*t++ = c;
 	}
 
 	/*
-	 * 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 */
         if (length < 1)
                 goto trunc;
@@ -1435,17 +1440,15 @@ ppp_hdlc(netdissect_options *ndo,
                 handle_ppp(ndo, proto, b + 2, length - 2);
             break;
         }
 
 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);
 }
 
 
 /* PPP */
diff --git a/print.c b/print.c
index b9ba5997d7..f20633388b 100644
--- a/print.c
+++ b/print.c
@@ -429,14 +429,18 @@ pretty_print_packet(netdissect_options *ndo, const struct pcap_pkthdr *h,
 	 * before it popped the packet information.
 	 */
 	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.
 		 */
 		if (ndo->ndo_Xflag > 1) {

Vulnerable code: 	const void *sb, *se;
	b = (u_char *)nd_malloc(ndo, caplen);
	if (b == NULL)
		return;
	 * Change the end pointer, so bounds checks work.
	 * Change the pointer to packet data to help debugging.
	sb = ndo->ndo_packetp;
	se = ndo->ndo_snapend;
	ndo->ndo_packetp = b;
	ndo->ndo_snapend = t;
	ndo->ndo_packetp = sb;
	ndo->ndo_snapend = se;
	ndo->ndo_packetp = sb;
	ndo->ndo_snapend = se;
	 * Restore the original snapend, as a printer might have
	 * changed it.
Fixed code: #include <stdlib.h>

	b = (u_char *)malloc(caplen);
	if (b == NULL) {
		(*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
			"%s: malloc", __func__);
	}
	 * 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.
	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__);
	}
	nd_pop_packet_info(ndo);
	nd_pop_packet_info(ndo);
	 * 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_packetp = sp;