If Curl_done is called with premature == TRUE we can't pipeline #690

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@Andersbakken
Contributor

This prevents a crash in the following scenario:

2 (or more) requests are made to the same host and pipelining is enabled.

The first request times out. Curl_done will return early in this block
since conn->send_pipe->size == 1:

if((conn->send_pipe->size + conn->recv_pipe->size != 0 &&
!data->set.reuse_forbid &&
!conn->bits.close)) {
/* Stop if pipeline is not empty and we do not have to close
connection. */
DEBUGF(infof(data, "Connection still in use, no more Curl_done now!\n"));
return CURLE_OK;
}

When the second one fails it will not return early and will in fact
call Curl_disconnect which will free the connection.

When curl_easy_cleanup is called on the first request his easy_conn
pointer will be a dangling pointer and the app will crash.

ASAN output:

==7245==ERROR: AddressSanitizer: heap-use-after-free on address 0xda366d80 at pc 0x986d05b bp 0xd94b16d8 sp 0xd94b16c0
READ of size 4 at 0xda366d80 thread T16 (RESOURCE_HTTP)
    #0 0x986d05a in curl_multi_remove_handle
    #1 0x98a3e2b in Curl_close
    #2 0x985abb5 in curl_easy_cleanup
    #3 0x9777302 in <application frames>
    #4 0xf7b2a016 in __asan::AsanThread::ThreadStart(unsigned long)
    #5 0xf7b16f40 in asan_thread_start(void*)
    #6 0xf7a971a9 in start_thread
    #7 0xf681b02d in clone

0xda366d80 is located 0 bytes inside of 1100-byte region [0xda366d80,0xda3671cc)
freed by thread T16 (RESOURCE_HTTP) here:
    #0 0xf7b1fa5a in free
    #1 0x98b5948 in Curl_disconnect
    #2 0x98d067d in Curl_done
    #3 0x9875a61 in curl_multi_perform
    #4 0x9758a71 in <application frames>
    #5 0xf7b2a016 in __asan::AsanThread::ThreadStart(unsigned long)
    #6 0xf681b02d in clone
@Andersbakken Andersbakken If Curl_done is called with premature == TRUE we can't pipeline
This prevents a crash in the following scenario:

2 (or more) requests are made to the same host and pipelining is enabled.

The first request times out. Curl_done will return early in this block
since conn->send_pipe->size == 1:

  if((conn->send_pipe->size + conn->recv_pipe->size != 0 &&
      !data->set.reuse_forbid &&
      !conn->bits.close)) {
    /* Stop if pipeline is not empty and we do not have to close
       connection. */
    DEBUGF(infof(data, "Connection still in use, no more Curl_done now!\n"));
    return CURLE_OK;
  }

When the second one fails it will not return early and will in fact
call Curl_disconnect which will free the connection.

When curl_easy_cleanup is called on the first request his easy_conn
pointer will be a dangling pointer and the app will crash.

ASAN output:

==7245==ERROR: AddressSanitizer: heap-use-after-free on address 0xda366d80 at pc 0x986d05b bp 0xd94b16d8 sp 0xd94b16c0
READ of size 4 at 0xda366d80 thread T16 (RESOURCE_HTTP)
    #0 0x986d05a in curl_multi_remove_handle
    #1 0x98a3e2b in Curl_close
    #2 0x985abb5 in curl_easy_cleanup
    #3 0x9777302 in <application frames>
    #4 0xf7b2a016 in __asan::AsanThread::ThreadStart(unsigned long)
    #5 0xf7b16f40 in asan_thread_start(void*)
    #6 0xf7a971a9 in start_thread
    #7 0xf681b02d in clone

0xda366d80 is located 0 bytes inside of 1100-byte region [0xda366d80,0xda3671cc)
freed by thread T16 (RESOURCE_HTTP) here:
    #0 0xf7b1fa5a in free
    #1 0x98b5948 in Curl_disconnect
    #2 0x98d067d in Curl_done
    #3 0x9875a61 in curl_multi_perform
    #4 0x9758a71 in <application frames>
    #5 0xf7b2a016 in __asan::AsanThread::ThreadStart(unsigned long)
    #6 0xf681b02d in clone
4d28e4a
@jay jay added a commit that referenced this pull request Mar 1, 2016
@Andersbakken @jay Andersbakken + jay url: if Curl_done is premature then pipeline not in use
Prevent a crash if 2 (or more) requests are made to the same host and
pipelining is enabled and the connection does not complete.

Bug: #690
3c2ef2a
@jay
Member
jay commented Mar 2, 2016

Thanks, landed in 3c2ef2a.

@jay jay closed this Mar 2, 2016
@Andersbakken
Contributor

Thanks.
On Tue, Mar 1, 2016 at 4:01 PM Jay Satiro notifications@github.com wrote:

Closed #690 #690.


Reply to this email directly or view it on GitHub
#690 (comment).

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