Assertion failure in bio_cf_out_write if CURLOPT_HEADERFUNCTION callback gets CURLINFO_TLS_SSL_PTR

[fujii]

I'm using libcurl 7.87.0 with libressl 3.7.0 for Windows WebKit.
After upgrading libcurl to 7.87.0 from 7.86, WebKit browser is crashing by loading https pages with a proxy.

With the debug libcurl.dll, I'm observing the following assertion failure.

Assertion failed!

Program: ...bkit\gb\WebKitLibraries\wincairo\bin64\libcurl.dll
File: D:\work\WinCairoReq\g\buildtrees\curl\src\7...\openssl.c
Line: 709

Expression: data

Callstack:

ucrtbased.dll!common_assert_to_message_box<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number, void * const return_address) Line 388	C++
ucrtbased.dll!common_assert<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number, void * const return_address) Line 424	C++
ucrtbased.dll!_wassert(const wchar_t * expression, const wchar_t * file_name, unsigned int line_number) Line 444	C++
libcurl.dll!bio_cf_out_write(bio_st * bio, const char * buf, int blen) Line 709	C
crypto-50.dll!BIO_write(bio_st * b, const void * in, int inl) Line 412	C
ssl-53.dll!tls13_legacy_wire_write(ssl_st * ssl, const unsigned char * buf, unsigned __int64 len) Line 75	C
ssl-53.dll!tls13_legacy_wire_write_cb(const void * buf, unsigned __int64 n, void * arg) Line 97	C
ssl-53.dll!tls13_record_send(tls13_record * rec, __int64(*)(const void *, unsigned __int64, void *) wire_write, void * wire_arg) Line 178	C
ssl-53.dll!tls13_record_layer_write_record(tls13_record_layer * rl, unsigned char content_type, const unsigned char * content, unsigned __int64 content_len) Line 1082	C
ssl-53.dll!tls13_record_layer_write_chunk(tls13_record_layer * rl, unsigned char content_type, const unsigned char * buf, unsigned __int64 n) Line 1101	C
ssl-53.dll!tls13_record_layer_write(tls13_record_layer * rl, unsigned char content_type, const unsigned char * buf, unsigned __int64 n) Line 1116	C
ssl-53.dll!tls13_write_handshake_data(tls13_record_layer * rl, const unsigned char * buf, unsigned __int64 n) Line 1164	C
ssl-53.dll!tls13_handshake_msg_send(tls13_handshake_msg * msg, tls13_record_layer * rl) Line 180	C
ssl-53.dll!tls13_handshake_send_action(tls13_ctx * ctx, const tls13_handshake_action * action) Line 470	C
ssl-53.dll!tls13_handshake_perform(tls13_ctx * ctx) Line 410	C
ssl-53.dll!tls13_client_connect(tls13_ctx * ctx) Line 85	C
ssl-53.dll!tls13_legacy_connect(ssl_st * ssl) Line 462	C
ssl-53.dll!SSL_connect(ssl_st * s) Line 993	C
libcurl.dll!ossl_connect_step2(Curl_cfilter * cf, Curl_easy * data) Line 3901	C
libcurl.dll!ossl_connect_common(Curl_cfilter * cf, Curl_easy * data, bool nonblocking, bool * done) Line 4441	C
libcurl.dll!ossl_connect_nonblocking(Curl_cfilter * cf, Curl_easy * data, bool * done) Line 4475	C
libcurl.dll!ssl_connect_nonblocking(Curl_cfilter * cf, Curl_easy * data, bool * done) Line 358	C
libcurl.dll!ssl_cf_connect(Curl_cfilter * cf, Curl_easy * data, bool blocking, bool * done) Line 1526	C
libcurl.dll!Curl_conn_connect(Curl_easy * data, int sockindex, bool blocking, bool * done) Line 367	C
libcurl.dll!multi_runsingle(Curl_multi * multi, curltime * nowp, Curl_easy * data) Line 2070	C
libcurl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Line 2690	C
WebCore.dll!WebCore::CurlMultiHandle::perform(int & runningHandles) Line 281	C++
WebCore.dll!WebCore::CurlRequestScheduler::workerThread() Line 176	C++
WebCore.dll!WebCore::CurlRequestScheduler::startOrWakeUpThread::__l2::<lambda_1>::operator()() Line 99	C++
WebCore.dll!WTF::Detail::CallableWrapper<`WebCore::CurlRequestScheduler::startOrWakeUpThread'::`2'::<lambda_1>,void>::call() Line 53	C++
WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 83	C++
WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) Line 250	C++
WTF.dll!WTF::wtfThreadEntryPoint(void * data) Line 151	C++
ucrtbase.dll!thread_start<unsigned int (__cdecl*)(void *),1>()	Unknown
kernel32.dll!00007ff986977614()	Unknown
ntdll.dll!00007ff9870626a1()	Unknown

This assertion was added by this change.
55807e6
#9962

data is cleared because CURLOPT_HEADERFUNCTION callback calls curl_easy_getinfo with CURLINFO_TLS_SSL_PTR.

callstack of cf_ctx_set_data:

libcurl.dll!cf_ctx_set_data(Curl_cfilter * cf, Curl_easy * data) Line 325	C
libcurl.dll!Curl_ssl_get_internals(Curl_easy * data, int sockindex, CURLINFO info, int n) Line 1725	C
libcurl.dll!getinfo_slist(Curl_easy * data, CURLINFO info, curl_slist * * param_slistp) Line 536	C
libcurl.dll!Curl_getinfo(Curl_easy * data, CURLINFO info, ...) Line 604	C
libcurl.dll!curl_easy_getinfo(Curl_easy * data, CURLINFO info, ...) Line 816	C
WebCore.dll!WebCore::CurlHandle::sslConnection() Line 802	C++
WebCore.dll!WebCore::CurlHandle::certificateInfo() Line 944	C++
WebCore.dll!WebCore::CurlRequest::didReceiveHeader(WTF::String && header) Line 384	C++
WebCore.dll!WebCore::CurlRequest::didReceiveHeaderCallback(char * ptr, unsigned __int64 blockSize, unsigned __int64 numberOfBlocks, void * userData) Line 824	C++
libcurl.dll!chop_write(Curl_easy * data, int type, char * optr, unsigned __int64 olen) Line 633	C
libcurl.dll!Curl_client_write(Curl_easy * data, int type, char * ptr, unsigned __int64 len) Line 678	C
libcurl.dll!recv_CONNECT_resp(Curl_easy * data, connectdata * conn, tunnel_state * ts, bool * done) Line 583	C
libcurl.dll!CONNECT(Curl_cfilter * cf, Curl_easy * data, tunnel_state * ts) Line 991	C
libcurl.dll!http_proxy_cf_connect(Curl_cfilter * cf, Curl_easy * data, bool blocking, bool * done) Line 1084	C
libcurl.dll!ssl_cf_connect(Curl_cfilter * cf, Curl_easy * data, bool blocking, bool * done) Line 1512	C
libcurl.dll!Curl_conn_connect(Curl_easy * data, int sockindex, bool blocking, bool * done) Line 367	C
libcurl.dll!multi_runsingle(Curl_multi * multi, curltime * nowp, Curl_easy * data) Line 2070	C
libcurl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Line 2690	C
WebCore.dll!WebCore::CurlMultiHandle::perform(int & runningHandles) Line 281	C++
WebCore.dll!WebCore::CurlRequestScheduler::workerThread() Line 176	C++
WebCore.dll!WebCore::CurlRequestScheduler::startOrWakeUpThread::__l2::<lambda_1>::operator()() Line 99	C++
WebCore.dll!WTF::Detail::CallableWrapper<`WebCore::CurlRequestScheduler::startOrWakeUpThread'::`2'::<lambda_1>,void>::call() Line 53	C++
WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 83	C++
WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) Line 250	C++
WTF.dll!WTF::wtfThreadEntryPoint(void * data) Line 151	C++
ucrtbase.dll!thread_start<unsigned int (__cdecl*)(void *),1>()	Unknown
kernel32.dll!00007ff986977614()	Unknown
ntdll.dll!00007ff9870626a1()	Unknown

[fujii]

I confirmed the following patch solves the crash bug.

diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index dd0414ce7..889f6137d 100644
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -315,11 +315,15 @@ static void cf_ctx_free(struct ssl_connect_data *ctx)
   }
 }
 
-static void cf_ctx_set_data(struct Curl_cfilter *cf,
+static struct Curl_easy *cf_ctx_set_data(struct Curl_cfilter *cf,
                             struct Curl_easy *data)
 {
-  if(cf->ctx)
+  struct Curl_easy *old_data = NULL;
+  if(cf->ctx) {
+    old_data = ((struct ssl_connect_data *)cf->ctx)->call_data;
     ((struct ssl_connect_data *)cf->ctx)->call_data = data;
+  }
+  return old_data;
 }
 
 static CURLcode ssl_connect(struct Curl_cfilter *cf, struct Curl_easy *data)
@@ -1786,9 +1790,9 @@ void *Curl_ssl_get_internals(struct Curl_easy *data, int sockindex,
     /* get first filter in chain, if any is present */
     cf = Curl_ssl_cf_get_ssl(data->conn->cfilter[sockindex]);
     if(cf) {
-      cf_ctx_set_data(cf, data);
+      struct Curl_easy *old_data = cf_ctx_set_data(cf, data);
       result = Curl_ssl->get_internals(cf->ctx, info);
-      cf_ctx_set_data(cf, NULL);
+      cf_ctx_set_data(cf, old_data);
     }
   }
   return result;

[fujii]

I was trying to create a simple program to reproduce the crash.
But, no luck so far. Maybe, there is another unknown condition to reproduce.

There is only one reproducible step I know at the moment.

1. Download the prebuilt Windows WebKit binary (WebKit WinCairo port) and WebKitRequirements.
   https://trac.webkit.org/wiki/BuildingCairoOnWindows#DownloadbuildartifactsfromBuildbot
2. set proxy env vars

set http_proxy=http://your-proxy:8080/
set https_proxy=%http_proxy%

3. Start the browser
4. It crashes.

[jay]

@icing any idea what could be causing this?

[icing]

@fujii thanks for all the details. Your patch will indeed solve the issue. Thinking about a better way to address this.

@jay we have, via the user callbacks, a recursive call into the filters and the current way of handling the call_data, e.g. the currently relevant easy handle, will clear it before the "outer" invocation is done.

[jay]

Tricky. There's multi->in_callback if that helps.

edit: It's possible in_callback might not be set for all user callbacks, so maybe it won't help as much as I thought...

[icing]

Indeed. And possibly, those could happen nested as well.

[icing]

How about #10340 to address this?

[jay]

@fujii A potential fix for this issue just landed in 9e93bd4. Please let us know if you can still reproduce with the fix. Thanks

[fujii]

I tested and confirmed it works fine. Thank you very much.