From: Azat Khuzhin Date: Mon, 1 Feb 2016 14:32:09 +0000 (+0300) Subject: evdns: name_parse(): fix remote stack overread X-Git-Tag: release-2.1.6-beta~77 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=96f64a022014a208105ead6c8a7066018449d86d;p=libevent 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 --- diff --git a/evdns.c b/evdns.c index 0955a289..c4112330 100644 --- a/evdns.c +++ b/evdns.c @@ -976,7 +976,6 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, int name_out_len) { for (;;) { u8 label_len; - if (j >= length) return -1; GET8(label_len); if (!label_len) break; if (label_len & 0xc0) { @@ -997,6 +996,7 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, int name_out_len) { *cp++ = '.'; } if (cp + label_len >= end) return -1; + if (j + label_len > length) return -1; memcpy(cp, packet + j, label_len); cp += label_len; j += label_len;