Skip to content

Commit b9811ef

Browse files
committed
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.
1 parent 244456a commit b9811ef

File tree

2 files changed

+23
-16
lines changed

2 files changed

+23
-16
lines changed

print-ppp.c

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737

3838
#include "netdissect-stdinc.h"
3939

40+
#include <stdlib.h>
41+
4042
#include "netdissect.h"
4143
#include "extract.h"
4244
#include "addrtoname.h"
@@ -1358,17 +1360,18 @@ ppp_hdlc(netdissect_options *ndo,
13581360
u_char *b, *t, c;
13591361
const u_char *s;
13601362
u_int i, proto;
1361-
const void *sb, *se;
13621363

13631364
if (caplen == 0)
13641365
return;
13651366

13661367
if (length == 0)
13671368
return;
13681369

1369-
b = (u_char *)nd_malloc(ndo, caplen);
1370-
if (b == NULL)
1371-
return;
1370+
b = (u_char *)malloc(caplen);
1371+
if (b == NULL) {
1372+
(*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
1373+
"%s: malloc", __func__);
1374+
}
13721375

13731376
/*
13741377
* Unescape all the data into a temporary, private, buffer.
@@ -1389,13 +1392,15 @@ ppp_hdlc(netdissect_options *ndo,
13891392
}
13901393

13911394
/*
1392-
* Change the end pointer, so bounds checks work.
1393-
* Change the pointer to packet data to help debugging.
1395+
* Switch to the output buffer for dissection, and save it
1396+
* on the buffer stack so it can be freed; our caller must
1397+
* pop it when done.
13941398
*/
1395-
sb = ndo->ndo_packetp;
1396-
se = ndo->ndo_snapend;
1397-
ndo->ndo_packetp = b;
1398-
ndo->ndo_snapend = t;
1399+
if (!nd_push_buffer(ndo, b, b, (u_int)(t - b))) {
1400+
free(b);
1401+
(*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
1402+
"%s: can't push buffer on buffer stack", __func__);
1403+
}
13991404
length = ND_BYTES_AVAILABLE_AFTER(b);
14001405

14011406
/* now lets guess about the payload codepoint format */
@@ -1437,13 +1442,11 @@ ppp_hdlc(netdissect_options *ndo,
14371442
}
14381443

14391444
cleanup:
1440-
ndo->ndo_packetp = sb;
1441-
ndo->ndo_snapend = se;
1445+
nd_pop_packet_info(ndo);
14421446
return;
14431447

14441448
trunc:
1445-
ndo->ndo_packetp = sb;
1446-
ndo->ndo_snapend = se;
1449+
nd_pop_packet_info(ndo);
14471450
nd_print_trunc(ndo);
14481451
}
14491452

print.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,14 @@ pretty_print_packet(netdissect_options *ndo, const struct pcap_pkthdr *h,
431431
nd_pop_all_packet_info(ndo);
432432

433433
/*
434-
* Restore the original snapend, as a printer might have
435-
* changed it.
434+
* Restore the originals snapend and packetp, as a printer
435+
* might have changed them.
436+
*
437+
* XXX - nd_pop_all_packet_info() should have restored the
438+
* original values, but, just in case....
436439
*/
437440
ndo->ndo_snapend = sp + h->caplen;
441+
ndo->ndo_packetp = sp;
438442
if (ndo->ndo_Xflag) {
439443
/*
440444
* Print the raw packet data in hex and ASCII.

0 commit comments

Comments
 (0)