Skip to content

Commit 96f64a0

Browse files
committed
evdns: name_parse(): fix remote stack overread
@asn-the-goblin-slayer: "the name_parse() function in libevent's DNS code is vulnerable to a buffer overread. 971 if (cp != name_out) { 972 if (cp + 1 >= end) return -1; 973 *cp++ = '.'; 974 } 975 if (cp + label_len >= end) return -1; 976 memcpy(cp, packet + j, label_len); 977 cp += label_len; 978 j += label_len; No check is made against length before the memcpy occurs. This was found through the Tor bug bounty program and the discovery should be credited to 'Guido Vranken'." Reproducer for gdb (https://gist.github.com/azat/e4fcf540e9b89ab86d02): set $PROT_NONE=0x0 set $PROT_READ=0x1 set $PROT_WRITE=0x2 set $MAP_ANONYMOUS=0x20 set $MAP_SHARED=0x01 set $MAP_FIXED=0x10 set $MAP_32BIT=0x40 start set $length=202 # overread set $length=2 # allocate with mmap to have a seg fault on page boundary set $l=(1<<20)*2 p mmap(0, $l, $PROT_READ|$PROT_WRITE, $MAP_ANONYMOUS|$MAP_SHARED|$MAP_32BIT, -1, 0) set $packet=(char *)$1+$l-$length # hack the packet set $packet[0]=63 set $packet[1]='/' p malloc(sizeof(int)) set $idx=(int *)$2 set $idx[0]=0 set $name_out_len=202 p malloc($name_out_len) set $name_out=$3 # have WRITE only mapping to fail on read set $end=$1+$l p (void *)mmap($end, 1<<12, $PROT_NONE, $MAP_ANONYMOUS|$MAP_SHARED|$MAP_FIXED|$MAP_32BIT, -1, 0) set $m=$4 p name_parse($packet, $length, $idx, $name_out, $name_out_len) x/2s (char *)$name_out Before this patch: $ gdb -ex 'source gdb' dns-example $1 = 1073741824 $2 = (void *) 0x633010 $3 = (void *) 0x633030 $4 = (void *) 0x40200000 Program received signal SIGSEGV, Segmentation fault. __memcpy_sse2_unaligned () at memcpy-sse2-unaligned.S:33 After this patch: $ gdb -ex 'source gdb' dns-example $1 = 1073741824 $2 = (void *) 0x633010 $3 = (void *) 0x633030 $4 = (void *) 0x40200000 $5 = -1 0x633030: "/" 0x633032: "" (gdb) p $m $6 = (void *) 0x40200000 (gdb) p $1 $7 = 1073741824 (gdb) p/x $1 $8 = 0x40000000 (gdb) quit P.S. plus drop one condition duplicate. Fixes: #317
1 parent 329acc1 commit 96f64a0

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

Diff for: evdns.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,6 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, int name_out_len) {
976976

977977
for (;;) {
978978
u8 label_len;
979-
if (j >= length) return -1;
980979
GET8(label_len);
981980
if (!label_len) break;
982981
if (label_len & 0xc0) {
@@ -997,6 +996,7 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, int name_out_len) {
997996
*cp++ = '.';
998997
}
999998
if (cp + label_len >= end) return -1;
999+
if (j + label_len > length) return -1;
10001000
memcpy(cp, packet + j, label_len);
10011001
cp += label_len;
10021002
j += label_len;

0 commit comments

Comments
 (0)