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

Segfault in imap_done when calling curl_multi_remove_handle() #1953

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

Segfault in imap_done when calling curl_multi_remove_handle() #1953

cmeister2 opened this issue Oct 5, 2017 · 7 comments
Labels
crash IMAP

Comments

@cmeister2
Copy link
Contributor

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.
@bagder
Copy link
Member

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.

@cmeister2
Copy link
Contributor Author 

#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.

@cmeister2
Copy link
Contributor Author

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

@cmeister2
Copy link
Contributor Author

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).

@cmeister2
Copy link
Contributor Author

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

@bagder
Copy link
Member

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
@bagder
pingpong: return error when trying to send without connection
a9a6759 
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
@bagder bagder mentioned this issue Oct 6, 2017
Closed
@cmeister2
Copy link
Contributor Author

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

@bagder bagder closed this as completed in 5b54df0 Oct 7, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
crash IMAP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pingpong: return error when trying to send without connection curl/curl
2 participants
@bagder @cmeister2