Segfault in imap_done when calling curl_multi_remove_handle() #1953

Closed
cmeister2 opened this Issue Oct 5, 2017 · 7 comments

Comments

Projects
None yet
2 participants
Contributor

cmeister2 commented Oct 5, 2017

I did this

Added curl_multi_remove_handle() to the fuzzer and checked it against CI.

I expected the following

Not to hit a segfault...
Broken build is here: https://travis-ci.org/curl/curl-fuzzer/jobs/283918193

Verbose logs add on some context:

root@kali:/src/curl-fuzzer# FUZZ_VERBOSE=yes ./curl_fuzzer curl_fuzz_data/oss-fuzz-gen-00070fdad78b9d907fce00d63d2933bd0328daa5 2>&1 | asan_symbolize
* WARNING: Using weak random seed
* STATE: INIT => CONNECT handle 0x62a000000208; line 1422 (connection #-5000)
* Added connection 0. The cache now contains 1 members
* STATE: CONNECT => WAITRESOLVE handle 0x62a000000208; line 1458 (connection #0)
* Expire cleared
* multi_done
ASAN:DEADLYSIGNAL
=================================================================
==18886==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000064ecc1 bp 0x7ffd338b4ff0 sp 0x7ffd338b4da0 T0)
==18886==The signal is caused by a READ memory access.
==18886==Hint: address points to the zero page.
    #0 0x64ecc0 in Curl_pp_vsendf /src/curl/lib/pingpong.c:171
    #1 0x64ecc0 in ?? ??:0
    #2 0x64f717 in Curl_pp_sendf /src/curl/lib/pingpong.c:255
    #3 0x64f717 in ?? ??:0
    #4 0x632eda in imap_done /src/curl/lib/imap.c:1467
    #5 0x632eda in ?? ??:0
    #6 0x52d82b in multi_done /src/curl/lib/multi.c:563
    #7 0x52d82b in ?? ??:0
    #8 0x52c8e6 in curl_multi_remove_handle /src/curl/lib/multi.c:725
    #9 0x52c8e6 in ?? ??:0
    #10 0x5173ae in _Z20fuzz_handle_transferP9fuzz_data /src/curl-fuzzer/curl_fuzzer.cc:710
    #11 0x5173ae in ?? ??:0
    #12 0x5136a7 in LLVMFuzzerTestOneInput /src/curl-fuzzer/curl_fuzzer.cc:89
    #13 0x5136a7 in ?? ??:0
    #14 0x6a4b6e in main /src/curl-fuzzer/standalone_fuzz_target_runner.cc:63
    #15 0x6a4b6e in ?? ??:0
    #16 0x7f4c114242b0 in __libc_start_main ??:?
    #17 0x7f4c114242b0 in ?? ??:0
    #18 0x41b7a9 in _start ??:?
    #19 0x41b7a9 in ?? ??:0

The issue appears to be calling this code:

      /* End the APPEND command first by sending an empty line */
      result = Curl_pp_sendf(&conn->proto.imapc.pp, "%s", "");

From gdb, pp.conn is NULL, and therefore we get a segfault when trying to deref a NULL pointer in Curl_pp_vsendf

(gdb) p conn->proto.imapc.pp 
$3 = {cache = 0x0, cache_size = 0, nread_resp = 0, linestart_resp = 0x0, pending_resp = false, 
  sendthis = 0x0, sendleft = 0, sendsize = 0, response = {tv_sec = 0, tv_usec = 0}, response_time = 0, 
  conn = 0x0, statemach_act = 0x0, endofresp = 0x0}

curl/libcurl version

devel

operating system

kali + ubuntu.

Owner

bagder commented Oct 6, 2017

In stack frame #4 where the Curl_pp_sendf function is called, can you check which of the struct fields a few lines above that made it go into that block of code?

The monster condition looks like:

  else if(!data->set.connect_only && !imap->custom &&
          (imap->uid || data->set.upload ||
          data->set.mimepost.kind != MIMEKIND_NONE)) {

So which of these was set to make the code execute this block? I'm trying to reproduce this with a test case but I don't quite understand the setup yet.

@bagder bagder added crash IMAP labels Oct 6, 2017

Contributor

cmeister2 commented Oct 6, 2017

#2  0x0000000000610ace in imap_done (conn=0x61c000000088, status=CURLE_OK, premature=true) at imap.c:1464
1464          result = Curl_pp_sendf(&conn->proto.imapc.pp, "%s", "");
(gdb) p data->set.connect_only
$1 = false
(gdb) p imap->custom
$2 = 0x0
(gdb) p imap->uid
$3 = 0x0
(gdb) p data->set.upload
$4 = false
(gdb) p data->set.mimepost.kind
$5 = MIMEKIND_MULTIPART

If you need more, let me know.

Contributor

cmeister2 commented Oct 6, 2017

in fact, here's a printout of imap and data in case it helps: https://gist.github.com/cmeister2/e0df6ecdeda744ba90a362fbec3b879b

Contributor

cmeister2 commented Oct 6, 2017

In case it's relevant:

 python read_corpus.py --input curl_fuzz_data/oss-fuzz-gen-00070fdad78b9d907fce00d63d2933bd0328daa5
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='curl_mime_addpart' (13), length=0, data='')
TLVHeader(type='CURLOPT_URL' (1), length=32, data='imap://127/(.0.1Content-Lehg:?n1')
TLVHeader(type='Server banner (sent on connection)' (2), length=71, data='* OK: \r\nbody\r\n\r-\n\xcd  y o \nA001  youa\n\r-\n+ \n  y\r-\n+ \n  yg errs\x00\x06\x00\x00\x00\x06secre')

curl_mime_addpart calls this code:

    case TLV_TYPE_MIME_PART:
      if(fuzz->mime == NULL) {
        fuzz->mime = curl_mime_init(fuzz->easy);
      }

      fuzz->part = curl_mime_addpart(fuzz->mime);
      break;

(there's a bug here, but that's separate).

Contributor

cmeister2 commented Oct 6, 2017

Also of note is that with #1957 this doesn't reproduce; probably because we're not in WAITRESOLVE state?

Owner

bagder commented Oct 6, 2017

A note to make here is that it has a mime stream set, but upload is false...

bagder added a commit that referenced this issue Oct 6, 2017

pingpong: return error when trying to send without connection
When imap_done() got called before a connection is setup, it would try
to "finish up" and dereffed a NULL pointer.

Test case 1153 managed to reproduce. I had to actually use a host name
to try to resolve to slow it down, as using the normal local server IP
will make libcurl get a connection in the first curl_multi_perform()
loop and then the bug doesn't trigger.

Fixes #1953
Assisted-by: Max Dymond
Contributor

cmeister2 commented Oct 6, 2017

7f1140c appears to have triggered this behaviour in the fuzzer, which makes this PR a priority to get in.

@bagder bagder closed this in 5b54df0 Oct 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment