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.
Otherwise we can trigger incorrect callback, the simplest way to trigger this
is using http regression tests -- https_chunk_out, since all it do is:
evhttp_send_reply_end()
evbuffer_add()
do_write()
evhttp_write_buffer()
evcon->cb = cb
And indeed this is what happens:
(gdb) bt
#0 do_write (bev_ssl=0x738a90, atmost=16384) at bufferevent_openssl.c:717
#1 0x00000000004b69f7 in consider_writing (bev_ssl=0x738a90) at bufferevent_openssl.c:875
#2 0x00000000004b7386 in be_openssl_outbuf_cb (buf=0x7387b0, cbinfo=0x7fffffffd590, arg=0x738a90) at bufferevent_openssl.c:1147
#3 0x0000000000490100 in evbuffer_run_callbacks (buffer=0x7387b0, running_deferred=0) at buffer.c:508
#4 0x00000000004901e5 in evbuffer_invoke_callbacks_ (buffer=0x7387b0) at buffer.c:529
#5 0x0000000000493a30 in evbuffer_add (buf=0x7387b0, data_in=0x4ecfb2, datlen=5) at buffer.c:1803
#6 0x00000000004be2e3 in evhttp_send_reply_end (req=0x7371a0) at http.c:2794
#7 0x000000000045c407 in http_chunked_trickle_cb (fd=-1, events=1, arg=0x75aaf0) at regress_http.c:402
...
(gdb) p bev.writecb
$4 = (bufferevent_data_cb) 0x4ba17e <evhttp_write_cb>
$5 = (void *) 0x7379b0
(gdb) p (struct evhttp_connection *)bev.cbarg
$6 = (struct evhttp_connection *) 0x7379b0
(gdb) p $6->cb
$7 = (void (*)(struct evhttp_connection *, void *)) 0x0
And be_sock don't do like this anyway.
Fixes: https_chunk_out
Now it included by openssl, but nfter
openssl/openssl@master-post-reformat-1494-g6329b60 it will print warning
(apparently they dropped <string.h> from the generic headers).
By using bufferevent_enable() there will be no event for READ *or* WRITE if
they are not enabled before, and this patch reduces difference for
be_sock_enable/be_openssl_enable (handshake)
Using the following examples you can get changes between be_openssl and
be_sock:
$ function diff_addr()
{
eval diff -u $(printf "<(strip_addr %s) " "$@")
}
$ function strip_addr()
{
sed 's/0x[a-zA-Z0-9]*/0xFFFF/g' "$@"
}
$ EVENT_DEBUG_LOGGING_ALL= regress --verbose --no-fork +http/https_connection_retry 2> /tmp/https-retry.log >&2
$ EVENT_DEBUG_LOGGING_ALL= regress --verbose --no-fork +http/connection_retry 2> /tmp/http-retry.log >&2
$ diff_addr /tmp/http-retry.log /tmp/https-retry.log
This patchset adds some basic tests to cover some https cases:
$ regress +http/https_..
http/https_basic: [forking] OK
http/https_simple: [forking] OK
http/https_simple_dirty: [forking] OK
http/https_incomplete: [forking] OK
http/https_incomplete_timeout: [forking] OK
http/https_connection_retry: [forking] OK
http/https_connection_retry_conn_address: [forking] OK
7 tests ok. (0 skipped)
But there are some leaks in http regression tests (like init_ssl() and others),
must be fixed by using custom setup routine.
* https-coverage-v6:
test/http: allow dirty shutdown for ssl to fix https_incomplete
test/http: https basic
test/http: incomplete{,_timeout} for https
test/http: add simplest test for http/https/https_dirty_shutdown
test/http: https: retry coverage
test/http: https server support (plus some helpers)
test/http: more sanity checks
test/ssl: export getkey()/getcert()/get_ssl_ctx()/init_ssl() for https
Fixes:
../test/regress_bufferevent.c: In function ‘test_bufferevent_socket_filter_inactive’:
../test/regress_bufferevent.c:1180:1: warning: label ‘end’ defined but not used [-Wunused-label]
end:
