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

curl 7.77.0 regression: segfault in Curl_ssl_getsessionid() #7148

Closed
cmb69 opened this issue May 29, 2021 · 4 comments
Closed

curl 7.77.0 regression: segfault in Curl_ssl_getsessionid() #7148

cmb69 opened this issue May 29, 2021 · 4 comments

Comments

@cmb69
Copy link
Contributor

@cmb69 cmb69 commented May 29, 2021

I did this

I ran the PHP curl extension's test suite with libcurl 7.77.0.

I expected the following

I expected all tests to succed like with libcurl 7.76.1.

curl/libcurl version

curl 7.77.0 (x86_64-pc-win32) libcurl/7.77.0 OpenSSL/1.1.1k zlib/1.2.11 WinIDN libssh2/1.9.0 nghttp2/1.40.0
Release-Date: 2021-05-26
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI UnixSockets alt-svc libz

operating system

Windows 10


Instead of all tests passing, a single test case fails due to a segfault in Curl_ssl_getsessionid(), because &data->state.session == NULLin

check = &data->state.session[i];

This is most likely caused by the fix for CVE-2021-22901. Maybe it is sufficient to return TRUE if &data->state.session == NULL here, but maybe the root cause lies somewhere else.

FWIW, the stack backtrace:

>	php_curl.dll!Curl_ssl_getsessionid(Curl_easy * data, connectdata * conn, const bool isProxy, void * * ssl_sessionid, unsigned __int64 * idsize, int sockindex) Zeile 418	C
 	php_curl.dll!ossl_connect_step1(Curl_easy * data, connectdata * conn, int sockindex) Zeile 3236	C
 	php_curl.dll!ossl_connect_common(Curl_easy * data, connectdata * conn, int sockindex, bool nonblocking, bool * done) Zeile 4062	C
 	php_curl.dll!ossl_connect_nonblocking(Curl_easy * data, connectdata * conn, int sockindex, bool * done) Zeile 4149	C
 	php_curl.dll!Curl_ssl_connect_nonblocking(Curl_easy * data, connectdata * conn, int sockindex, bool * done) Zeile 345	C
 	[Inlineframe] php_curl.dll!https_connecting(Curl_easy *) Zeile 1591	C
 	php_curl.dll!Curl_http_connect(Curl_easy * data, bool * done) Zeile 1514	C
 	[Inlineframe] php_curl.dll!protocol_connect(Curl_easy *) Zeile 1631	C
 	php_curl.dll!multi_runsingle(Curl_multi * multi, curltime * nowp, Curl_easy * data) Zeile 1964	C
 	php_curl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Zeile 2526	C
 	php_curl.dll!zif_curl_multi_exec(_zend_execute_data * execute_data, _zval_struct * return_value) Zeile 227	C
 	php8_debug.dll!ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER(_zend_execute_data * execute_data) Zeile 1298	C
 	php8_debug.dll!execute_ex(_zend_execute_data * ex) Zeile 54335	C
 	php8_debug.dll!zend_execute(_zend_op_array * op_array, _zval_struct * return_value) Zeile 58877	C
 	php8_debug.dll!zend_execute_scripts(int type, _zval_struct * retval, int file_count, ...) Zeile 1681	C
 	php8_debug.dll!php_execute_script(_zend_file_handle * primary_file) Zeile 2501	C
 	php.exe!do_cli(int argc, char * * argv) Zeile 951	C
 	php.exe!main(int argc, char * * argv) Zeile 1336	C
 	[Externer Code]	
@bagder
Copy link
Member

@bagder bagder commented May 29, 2021

I suppose 7.70.0 was a typo and you meant 7.77.0 ?

Any chance you can convert that into a stand-alone C app that reproduces the problem?

@cmb69 cmb69 changed the title curl 7.70.0 regression: segfault in Curl_ssl_getsessionid() curl 7.77.0 regression: segfault in Curl_ssl_getsessionid() May 29, 2021
@cmb69
Copy link
Contributor Author

@cmb69 cmb69 commented May 29, 2021

I suppose 7.70.0 was a typo and you meant 7.77.0 ?

Right! I've edited the versions above.

Any chance you can convert that into a stand-alone C app that reproduces the problem?

I'll give that a try.

@cmb69
Copy link
Contributor Author

@cmb69 cmb69 commented May 31, 2021

C reproducer:

#include <curl/curl.h>

int transfers = 1;

size_t write_callback(char *ptr, size_t size, size_t nmemb, void *userdata)
{
    printf("Received %zu", nmemb);
    return nmemb;
}

int callback(CURL *parent,
             CURL *easy,
             size_t num_headers,
             struct curl_pushheaders *headers,
             void *userp)
{
    curl_easy_setopt(easy, CURLOPT_WRITEFUNCTION, write_callback);
    transfers++;
    return CURL_PUSH_OK;
}

int main()
{
    CURLM *mh = curl_multi_init();
    curl_multi_setopt(mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
    curl_multi_setopt(mh, CURLMOPT_PUSHFUNCTION, callback);
    CURL *h = curl_easy_init();
    curl_easy_setopt(h, CURLOPT_URL, "https://http2.golang.org/serverpush");
    curl_easy_setopt(h, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
    curl_easy_setopt(h, CURLOPT_SSL_VERIFYHOST, 0);
    curl_easy_setopt(h, CURLOPT_SSL_VERIFYPEER, 0);
    curl_multi_add_handle(mh, h);
    int running;
    do {
        curl_multi_perform(mh, &running);
        CURLMsg *info;
        do {
            int msgs_in_queue;
            info = curl_multi_info_read(mh, &msgs_in_queue);
            if (info && info->msg == CURLMSG_DONE) {
                CURL *handle = info->easy_handle;
                if (handle) {
                    transfers--;
                    curl_multi_remove_handle(mh, handle);
                    curl_easy_cleanup(handle);
                }
            }
        } while (info);
    } while (transfers);
    curl_multi_cleanup(mh);

    return 0;
}

Pretty contrived, but worked with cURL 7.76.1.

bagder added a commit that referenced this issue May 31, 2021
This function might get called for an easy handle for which the session
cache hasn't been setup. It now just returns a "miss" in that case.

Fixes #7148
bagder added a commit that referenced this issue May 31, 2021
Debug builds would warn that these structs were not initialized properly
for pushed streams.

Ref: #7148
@bagder
Copy link
Member

@bagder bagder commented May 31, 2021

Thanks, this made it really easy to reproduce!

@bagder bagder closed this in 894c747 May 31, 2021
bagder added a commit that referenced this issue May 31, 2021
Debug builds would warn that these structs were not initialized properly
for pushed streams.

Ref: #7148
Closes #7153
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants