Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doh: memory leak after followlocation #4592

Closed
pauldreik opened this issue Nov 13, 2019 · 3 comments
Closed

doh: memory leak after followlocation #4592

pauldreik opened this issue Nov 13, 2019 · 3 comments
Labels
KNOWN_BUGS material memory-leak name lookup

Comments

@pauldreik
Copy link
Contributor

@pauldreik pauldreik commented Nov 13, 2019

Using the same setup as in #4463 the following testcase gives a leak with the custom fuzzer.
The curl version used is pauldreik@5dcc3e5, which has 674298d merged (curl master as of 20191113)

The input is the following (pick your favourite presentation variant :-)

paul@ink:~/code/delaktig/curl/build-fuzz-clang8-asan-ubsan$ ../../curl-fuzzer/read_corpus.py --input leak
TLVHeader(type='CURLOPT_URL' (1), length=1, data='\xff')
TLVHeader(type='Server banner (sent on connection)' (2), length=21, data='HTTP/3301\nLocation:\xff\n')
TLVHeader(type='CURLOPT_FOLLOWLOCATION' (29), length=4, data='    ')
TLVHeader(type='CURLOPT_DOH_URL' (41), length=45, data='127.0.1.127\x00                                 ')
2019-11-13 17:47:58,233 INFO  Returning 0

paul@ink:~/code/delaktig/curl/build-fuzz-clang8-asan-ubsan$ hd leak 
00000000  00 01 00 00 00 01 ff 00  02 00 00 00 15 48 54 54  |.............HTT|
00000010  50 2f 33 33 30 31 0a 4c  6f 63 61 74 69 6f 6e 3a  |P/3301.Location:|
00000020  ff 0a 00 1d 00 00 00 04  20 20 20 20 00 29 00 00  |........    .)..|
00000030  00 2d 31 32 37 2e 30 2e  31 2e 31 32 37 00 20 20  |.-127.0.1.127.  |
00000040  20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |
*
0000005f

paul@ink:~/code/delaktig/curl/build-fuzz-clang8-asan-ubsan$ base64 <leak 
AAEAAAAB/wACAAAAFUhUVFAvMzMwMQpMb2NhdGlvbjr/CgAdAAAABCAgICAAKQAAAC0xMjcuMC4x
LjEyNwAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICA=

Here is the trace from libfuzzer:

paul@ink:~/code/delaktig/curl/build-fuzz-clang8-asan-ubsan$ tests/curl_fuzzer_http_wcm  leak
INFO: Seed: 4285494780
INFO: Loaded 1 modules   (314943 inline 8-bit counters): 314943 [0x22b0ab0, 0x22fd8ef),
INFO: Loaded 1 PC tables (314943 PCs): 314943 [0x1899a50,0x1d67e40),
tests/curl_fuzzer_http_wcm: Running 1 inputs 1 time(s) each.
Running: leak

=================================================================

==28765==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x565933 in __interceptor_malloc (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x565933)
    #1 0x5b9005 in curl_dbg_malloc /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/memdebug.c:174:9
    #2 0x66eb3e in Curl_slist_append_nodup /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/slist.c:66:14
    #3 0x66f3bf in curl_slist_append /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/slist.c:97:10
    #4 0xb454f0 in Curl_doh /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/doh.c:376:5
    #5 0x7cc5be in Curl_resolv /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/hostip.c:535:14
    #6 0x7cd9c6 in Curl_resolv_timeout /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/hostip.c:722:8
    #7 0x92ab1d in resolve_server /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3147:12
    #8 0x8e3e06 in create_conn /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3765:12
    #9 0x8ca110 in Curl_connect /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3869:12
    #10 0x610cb0 in multi_runsingle /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/multi.c:1511:16
    #11 0x60b1c5 in curl_multi_perform /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/multi.c:2226:14
    #12 0x59a815 in fuzz_handle_transfer(FUZZ_DATA*) /home/paul/code/delaktig/curl-fuzzer/intree_fuzzer/src/networkfuzzers/curl_fuzzer.cc:210:3
    #13 0x598b2f in LLVMFuzzerTestOneInput /home/paul/code/delaktig/curl-fuzzer/intree_fuzzer/src/networkfuzzers/curl_fuzzer.cc:112:3
    #14 0x49fc5a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x49fc5a)
    #15 0x49259c in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x49259c)
    #16 0x497a21 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x497a21)
    #17 0x4ba042 in main (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x4ba042)
    #18 0x7efe9fd3009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Indirect leak of 54 byte(s) in 1 object(s) allocated from:
    #0 0x565933 in __interceptor_malloc (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x565933)
    #1 0x5b9005 in curl_dbg_malloc /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/memdebug.c:174:9
    #2 0x5ba3cd in curl_dbg_strdup /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/memdebug.c:229:9
    #3 0x66f37f in curl_slist_append /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/slist.c:92:19
    #4 0xb454f0 in Curl_doh /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/doh.c:376:5
    #5 0x7cc5be in Curl_resolv /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/hostip.c:535:14
    #6 0x7cd9c6 in Curl_resolv_timeout /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/hostip.c:722:8
    #7 0x92ab1d in resolve_server /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3147:12
    #8 0x8e3e06 in create_conn /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3765:12
    #9 0x8ca110 in Curl_connect /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/url.c:3869:12
    #10 0x610cb0 in multi_runsingle /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/multi.c:1511:16
    #11 0x60b1c5 in curl_multi_perform /home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/../lib/multi.c:2226:14
    #12 0x59a815 in fuzz_handle_transfer(FUZZ_DATA*) /home/paul/code/delaktig/curl-fuzzer/intree_fuzzer/src/networkfuzzers/curl_fuzzer.cc:210:3
    #13 0x598b2f in LLVMFuzzerTestOneInput /home/paul/code/delaktig/curl-fuzzer/intree_fuzzer/src/networkfuzzers/curl_fuzzer.cc:112:3
    #14 0x49fc5a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x49fc5a)
    #15 0x49259c in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x49259c)
    #16 0x497a21 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x497a21)
    #17 0x4ba042 in main (/home/paul/code/delaktig/curl/build-fuzz-clang8-asan-ubsan/tests/curl_fuzzer_http_wcm+0x4ba042)
    #18 0x7efe9fd3009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: 86 byte(s) leaked in 2 allocation(s).
@bagder bagder added memory-leak name lookup labels Nov 14, 2019
@bagder
Copy link
Member

@bagder bagder commented Dec 16, 2019

A little bit of a blind guess here but it goes in line with the fix for the previous DoH leak as I believe this is similar. Does this patch fix it?

diff --git a/lib/url.c b/lib/url.c
index 4111eec3a..5f272a715 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -1987,10 +1987,11 @@ void Curl_free_request_state(struct Curl_easy *data)
   Curl_safefree(data->req.newurl);
 
 #ifndef CURL_DISABLE_DOH
   Curl_close(&data->req.doh.probe[0].easy);
   Curl_close(&data->req.doh.probe[1].easy);
+  curl_slist_free_all(data->req.doh.headers);
 #endif
 }
 
 
 #ifndef CURL_DISABLE_PROXY

@pauldreik
Copy link
Contributor Author

@pauldreik pauldreik commented Dec 21, 2019

No, the patch actually worsens the situation into a use after free...
Adding a

data->req.doh.headers=NULL;

to your patch fixes this particular test case, but causes leaks for other fuzz cases.

@bagder
Copy link
Member

@bagder bagder commented Jan 9, 2020

I cannot repro this using normal curl/libcurl with DoH and redirects. That makes me not too concerned about this issue. Test case:

$ curl daniel.haxx.se/test/redir2.cgi --doh-url https://1.1.1.1/dns-query -L

@bagder bagder closed this as completed in 34696ab Mar 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
KNOWN_BUGS material memory-leak name lookup
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bagder @pauldreik