@asn-the-goblin-slayer:
"Length between '[' and ']' is cast to signed 32 bit integer on line 1815. Is
the length is more than 2<<31 (INT_MAX), len will hold a negative value.
Consequently, it will pass the check at line 1816. Segfault happens at line
1819.
Generate a resolv.conf with generate-resolv.conf, then compile and run
poc.c. See entry-functions.txt for functions in tor that might be
vulnerable.
Please credit 'Guido Vranken' for this discovery through the Tor bug bounty
program."
Reproducer for gdb (https://gist.github.com/azat/be2b0d5e9417ba0dfe2c):
start
p (1ULL<<31)+1ULL
# $1 = 2147483649
p malloc(sizeof(struct sockaddr))
# $2 = (void *) 0x646010
p malloc(sizeof(int))
# $3 = (void *) 0x646030
p malloc($1)
# $4 = (void *) 0x7fff76a2a010
p memset($4, 1, $1)
# $5 = 1990369296
p (char *)$4
# $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
set $6[0]='['
set $6[$1]=']'
p evutil_parse_sockaddr_port($4, $2, $3)
# $7 = -1
Before:
$ gdb bin/http-connect < gdb
(gdb) $1 = 2147483649
(gdb) (gdb) $2 = (void *) 0x646010
(gdb) (gdb) $3 = (void *) 0x646030
(gdb) (gdb) $4 = (void *) 0x7fff76a2a010
(gdb) (gdb) $5 = 1990369296
(gdb) (gdb) $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
(gdb) (gdb) (gdb) (gdb)
Program received signal SIGSEGV, Segmentation fault.
__memcpy_sse2_unaligned () at memcpy-sse2-unaligned.S:36
After:
$ gdb bin/http-connect < gdb
(gdb) $1 = 2147483649
(gdb) (gdb) $2 = (void *) 0x646010
(gdb) (gdb) $3 = (void *) 0x646030
(gdb) (gdb) $4 = (void *) 0x7fff76a2a010
(gdb) (gdb) $5 = 1990369296
(gdb) (gdb) $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
(gdb) (gdb) (gdb) (gdb) $7 = -1
(gdb) (gdb) quit
Fixes: #318
Otherwise that #ifdef in visibility.h is useless, and __declspec(dllimport)
will be always on.
Fixes: #314
Fixes: 4545fa9b6866df47ce2f908631a84477a94d5f49 ("Add option to build shared
library")
Since ssize_it is POSIX, windows/VS also have this but with BaseTsd.h, plus the
logic prefers "ssize_t" (lower) instead of "SSIZE_T" (upper) when the latest
only available -- fix this too.
Refs: #311
Since we have some issues (see refs) for changing waiting order in event_del()
I wrote this simple test, so maybe this test can explain something or at least
cover what we have before and show it will be broken.
P.S. we really need avoid such stuff like lets-test-with-sleep/usleep.
Refs: #225
Refs: #226
Refs: #236
* event_reinit-for-signals-v3:
test/regress: cover existing signal callbacks and fork() + event_reinit()
test/regress: cover signals after fork() + event_reinit()
test/regress: main/fork: rewrite assertions by just removing event in callback
event_reinit: make signals works after fork() without evsig_add()
event_reinit: always re-init signal's socketpair
Fixes#307
Instead of assigning some variable value (got_child), and schedule exit from
loop from that callback, just remove event for that signal, and event loop will
exit automatically when there will be no events.
event_reinit() removes the event, but only evsig_add puts it back. So any
signals set up before event_reinit will be ignored until another signal is
added.
Fixes: #307
Before this patch event_reinit() only closes the signal socketpair fds and
recreates them if signals have been added, but this is wrong, since socketpair
fds created on backend init, and if we will not re-create them bad things in
child/parent signal handling will happens (and indeed this is what happens for
non-reinit backends like select).
Fixes: #307
Under the hood it's an unsigned rather than a signed type and whilst C
compilers are largely happy with this C++ compilers tend to be fussy
about class function signatures which makes C++ usage of libevent
problematic.
If you use bufferevent_socket_connect_hostname() to resolve, then ipv4 answer
can be returned before ipv6 scheduled and if you will destroy bufferevent after
ipv4 answer will come (in a separate thread of course) then ipv6 will trigger
UAF:
$ a.out
=================================================================
==29733==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000ef50 at pc 0x0000004b7aef bp 0x7fffffffd940 sp 0x7fffffffd0f8
READ of size 2 at 0x60200000ef50 thread T0
#0 0x4b7aee in __interceptor_index (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x4b7aee)
#1 0x5060eb in string_num_dots /src/oss/libevent/libevent-github/.cmake-debug/../evdns.c:2739
#2 0x5078df in search_request_new /src/oss/libevent/libevent-github/.cmake-debug/../evdns.c:3214
#3 0x506afd in evdns_base_resolve_ipv6 /src/oss/libevent/libevent-github/.cmake-debug/../evdns.c:2935
#4 0x50aa94 in evdns_getaddrinfo /src/oss/libevent/libevent-github/.cmake-debug/../evdns.c:4719
#5 0x51de4f in evutil_getaddrinfo_async_ /src/oss/libevent/libevent-github/.cmake-debug/../evutil.c:1567
#6 0x4fe023 in bufferevent_socket_connect_hostname /src/oss/libevent/libevent-github/.cmake-debug/../bufferevent_sock.c:519
#7 0x524f54 in evhttp_connection_connect_ /src/oss/libevent/libevent-github/.cmake-debug/../http.c:2493
#8 0x525156 in evhttp_make_request /src/oss/libevent/libevent-github/.cmake-debug/../http.c:2548
#9 0x52d373 in main (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x52d373)
#10 0x7ffff6849b44 in __libc_start_main /tmp/buildd/glibc-2.19/csu/libc-start.c:287
#11 0x445806 in _start (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x445806)
0x60200000ef50 is located 0 bytes inside of 15-byte region [0x60200000ef50,0x60200000ef5f)
freed by thread T1 here:
#0 0x4cc4f2 in __interceptor_free (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x4cc4f2)
#1 0x5141c1 in event_mm_free_ /src/oss/libevent/libevent-github/.cmake-debug/../event.c:3512
#2 0x522402 in evhttp_connection_free /src/oss/libevent/libevent-github/.cmake-debug/../http.c:1206
#3 0x52cc5f in connection_closer (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x52cc5f)
#4 0x50e80e in event_process_active_single_queue /src/oss/libevent/libevent-github/.cmake-debug/../event.c:1642
#5 0x50ed57 in event_process_active /src/oss/libevent/libevent-github/.cmake-debug/../event.c:1734
#6 0x50f458 in event_base_loop /src/oss/libevent/libevent-github/.cmake-debug/../event.c:1957
#7 0x50eddf in event_base_dispatch /src/oss/libevent/libevent-github/.cmake-debug/../event.c:1768
#8 0x52d075 in event_dispatch_thread (/src/oss/libevent/libevent-github/.invest/217-evhttp-threaded/a.out+0x52d075)
#9 0x7ffff74fc0a3 in start_thread /tmp/buildd/glibc-2.19/nptl/pthread_create.c:309
Fixes: #217Closes: #222Closes: #219
Gist: https://gist.github.com/azat/92cbb34232ac02d7972b (from #217 but thread-safe)
The mentioned commit adds this check under-the-else of the previous condition
between getaddrinfo()/gethostbyname_r(), so this check is triggered only when
we don't have getaddrinfo() which is wrong, fix this by move it upper.
Fixes [CI] since it uses getservbyname() and it failed with autotools only
(cmake detection is ok).
CI: https://travis-ci.org/libevent/libevent/builds/93125954
Fixes: af08a94085e49e6942835b4c6b50a774536d5b5b ("Check for getservbyname even
if not on win32.")
This patchset fixes some issues wit evdns_base_free(..., fail_requests=1), and
there are two cases: with callback wrapper for evdns_getaddrinfo() and not,
both fixed by this patches, and adds regression tests for this.
* evdns-fail-requests-v6:
evdns: evdns_base_free(): fix UAF of evdns_base with @fail_requests
test/dns: cover evdns_getaddrinfo() and evdns_base_free() with @fail_requests
evdns: evdns_base_free(): free requests before namservers
test/dns: cover @fail_requests for evdns_base_free()
test/dns: more graceful coverage of @fail_requests
If you call evdns_base_free() with @fail_requests == 1, then it will defer
callback with DNS_ERR_SHUTDOWN, but that callback (internal) uses
data->evdns_base, but we already freed that evdns base, so we can't do
this, fix this by checking @result to DNS_ERR_SHUTDOWN.
Fixes: regress dns/client_fail_requests_getaddrinfo
Fixes: #269
Otherwise we will trigger next UAF:
$ valgrind --vgdb-error=1 regress --no-fork +dns/client_fail_requests
==24733== Memcheck, a memory error detector
==24733== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==24733== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==24733== Command: regress --no-fork +dns/client_fail_requests
==24733==
==24733==
==24733== TO DEBUG THIS PROCESS USING GDB: start GDB like this
==24733== /path/to/gdb regress
==24733== and then give GDB the following command
==24733== target remote | /usr/lib/valgrind/../../bin/vgdb --pid=24733
==24733== --pid is optional if only one valgrind process is running
==24733==
dns/client_fail_requests: ==24733== Invalid read of size 4
==24733== at 0x4C3352: request_finished (evdns.c:662)
==24733== by 0x4CC8B7: evdns_base_free_and_unlock (evdns.c:4048)
==24733== by 0x4CCAFD: evdns_base_free (evdns.c:4088)
==24733== by 0x458E95: dns_client_fail_requests_test (regress_dns.c:2039)
==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105)
==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252)
==24733== by 0x48F67E: tinytest_main (tinytest.c:434)
==24733== by 0x47C0DA: main (regress_main.c:461)
==24733== Address 0x61e6f70 is 448 bytes inside a block of size 456 free'd
==24733== at 0x4C29EAB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24733== by 0x4A8F4D: event_mm_free_ (event.c:3512)
==24733== by 0x4CC7A1: evdns_nameserver_free (evdns.c:4021)
==24733== by 0x4CC7DC: evdns_base_free_and_unlock (evdns.c:4037)
==24733== by 0x4CCAFD: evdns_base_free (evdns.c:4088)
==24733== by 0x458E95: dns_client_fail_requests_test (regress_dns.c:2039)
==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105)
==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252)
==24733== by 0x48F67E: tinytest_main (tinytest.c:434)
==24733== by 0x47C0DA: main (regress_main.c:461)
==24733== Block was alloc'd at
==24733== at 0x4C28C4F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24733== by 0x4A8D5A: event_mm_malloc_ (event.c:3437)
==24733== by 0x4C8B96: evdns_nameserver_add_impl_ (evdns.c:2505)
==24733== by 0x4C916D: evdns_base_nameserver_ip_add (evdns.c:2629)
==24733== by 0x458DA3: dns_client_fail_requests_test (regress_dns.c:2031)
==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105)
==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252)
==24733== by 0x48F67E: tinytest_main (tinytest.c:434)
==24733== by 0x47C0DA: main (regress_main.c:461)
==24733==
==24733== (action on error) vgdb me ...
Fixes: regress dns/client_fail_requests
Fixes: #269
In case when evdns_base_free() called with @fail_requests, we can potentially
have leaks, but we can avoid them if we will run event loop once again to
trigger defer cbs, so let's do this, instead of magical decrements (and also
this will give an example how to avoid leaks for evdns).
This covers SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE error codes from ssl,
under which we must block read/write to avoid busy looping, and hence extra CPU
usage.
This test introduces custom BIO that will count read/write and validates
counters, with patches for be_openssl that drops handling
SSL/SSL_ERROR_WANT_READ there are more then 43K reads, so 100 is pretty ok.
