Skip to content

Curl 7.76.0 segfault during connection close (SFTP + HTTPPROXYTUNNEL + HTTPS proxy) #6898

Closed

Description

@mofarrell

I did this

#include <stdio.h>
#include <curl/curl.h>

int main() {

  CURL *curl;
  CURLcode res;

  curl_global_init(CURL_GLOBAL_DEFAULT);

  curl = curl_easy_init();
  if(curl) {

    curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
    curl_easy_setopt(curl, CURLOPT_HTTPPROXYTUNNEL, 1);
    curl_easy_setopt(curl, CURLOPT_PROXY, "proxy.example.com");
    curl_easy_setopt(curl, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
    curl_easy_setopt(curl, CURLOPT_PROXY_SSLCERT, "/var/some/file");
    curl_easy_setopt(curl, CURLOPT_PROXY_SSLKEY, "/var/some/file");
    curl_easy_setopt(curl, CURLOPT_PROXY_CAINFO, "/var/some/other/file");
    curl_easy_setopt(curl, CURLOPT_URL, "sftp://somesftpthing.com");
    curl_easy_setopt(curl, CURLOPT_USERPWD, "somesuer:somepassword");

    res = curl_easy_perform(curl);
    /* Check for errors */
    if(res != CURLE_OK) {
        fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(res));
    }

    curl_easy_cleanup(curl);
  }
  curl_global_cleanup();
  return 0;
}

Curl segfaults during shutdown with a null connection:
https://github.com/curl/curl/blob/master/lib/vssh/libssh2.c#L3030

Stacktrace:

#0 0x7ff626e2b657 in ssh_tls_send /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:3037:12
#1 0x7ff626c26491 in _libssh2_transport_send /libssh2/1.9.0/src/libssh2-1.9.0/src/transport.c:888:11
#2 0x7ff626c08d28 in channel_send_eof /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2253:10
#3 0x7ff626c08d28 in _libssh2_channel_close /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2406:14
#4 0x7ff626c09190 in _libssh2_channel_free /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2583:14
#5 0x7ff626c1d6fd in sftp_shutdown /libssh2/1.9.0/src/libssh2-1.9.0/src/sftp.c:1079:10
#6 0x7ff626c1d6fd in libssh2_sftp_shutdown /libssh2/1.9.0/src/libssh2-1.9.0/src/sftp.c:1093:5
#7 0x7ff626e2c18c in ssh_statemach_act /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2488:14
#8 0x7ff626e2fd94 in ssh_block_statemach.constprop.0 /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2944:14
#9 0x7ff626e176b3 in Curl_disconnect /curl/7.76.0/src/curl-7.76.0/lib/url.c:851:5
#10 0x7ff626dd163f in Curl_conncache_close_all_connections /curl/7.76.0/src/curl-7.76.0/lib/conncache.c:553:11
#11 0x7ff626e01d28 in curl_multi_cleanup /curl/7.76.0/src/curl-7.76.0/lib/multi.c:2471:5
#12 0x7ff626e183e9 in Curl_close /curl/7.76.0/src/curl-7.76.0/lib/url.c:379:5
#13 0x7ff626ddc399 in curl_easy_cleanup /curl/7.76.0/src/curl-7.76.0/lib/easy.c:742:3

I expected the following

Curl to not crash during shutdown

curl/libcurl version

curl 7.76.0 (x86_64-facebook-linux-gnu) libcurl/7.76.0 OpenSSL/1.1.1k zlib/1.2.8 c-ares/1.13.0 libssh2/1.9.0 nghttp2/1.33.0
Release-Date: 2021-03-31
Protocols: file ftp ftps http https ldap ldaps mqtt rtsp scp sftp 
Features: alt-svc AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets

Known working in:

curl 7.72.0 (x86_64-facebook-linux-gnu) libcurl/7.72.0 OpenSSL/1.1.1k zlib/1.2.8 c-ares/1.13.0 libssh2/1.9.0 nghttp2/1.33.0
Release-Date: 2020-08-19
Protocols: file ftp ftps http https ldap ldaps rtsp scp sftp 
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets

operating system

CentOS 8

Activity

  1. bagder commented on Apr 17, 2021

    @bagder
    Member

    So how is the connection NULL? It gets a connection attached to the transfer in Curl_disconnect(), doesn't it? Where does it go wrong from there?

  2. mofarrell commented on Apr 19, 2021

    @mofarrell
    Author

    I spent a little time looking at a core, and here is what I have found:

    (gdb) frame 15
    #15 0x0000000000dbcc1d in Curl_close (datap=datap@entry=0x7fb6f1a470e8) at /curl/7.76.0/src/curl-7.76.0/lib/url.c:379
    (gdb) p data
    $35 = (struct Curl_easy *) 0x7fbdba7b2000
    (gdb) frame
    #12 0x0000000000ca2644 in Curl_disconnect (data=0x7fbcf127f000, conn=conn@entry=0x7fbad627ab00, dead_connection=dead_connection@entry=false) at /curl/7.76.0/src/curl-7.76.0/lib/url.c:851
    (gdb) p data->conn
    $3 = (struct connectdata *) 0x7fbad627ab00
    (gdb) frame
    #10 0x0000000005eefd2e in ssh_statemach_act (data=data@entry=0x7fbcf127f000, block=block@entry=0x7fb6f1a46a9f) at /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2488
    (gdb) frame
    #3  0x0000000005eef2a8 in ssh_tls_send (sock=38147, buffer=0x7fb8dc740288, length=48, flags=<optimized out>, abstract=<optimized out>) at /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:3030
    (gdb) p data
    $4 = (struct Curl_easy *) 0x7fbdba7b2000
    (gdb) p data->conn
    $6 = (struct connectdata *) 0x0
    (gdb) frame
    #9  libssh2_sftp_shutdown (sftp=0x7fbcefbf0a00) at sftp.c:1093
    (gdb) p sftp->channel->session->abstract
    $24 = (void *) 0x7fbdba7b2000
    

    The outer transfer in Curl_disconnect appears to be for the sftp connection, while the inner data, the one that is failing, appears to be the HTTPS proxy connection (inferred from looking at the contents of the Curl_easy struct).
    It seems that libssh is caching the struct Curl_easy * for the proxy transfer in their session struct. This is not the transfer that gets a connection reattached during disconnection. The transfer getting the connection reattached appears to be the sftp transfer? As you can probably tell, I don't know how these tunneling requests are implemented, and I am not very familiar with the curl internals.

    FWIW I am guessing 215db086e09665ee7af9b646ad6c4d6e281001ac is the source of the regression . It introduced a detaching of a connection during Curl_close prior to running the cleanup. In this case, this clears the connection for the proxy transfer across the cleanup call.

  3. bagder commented on Apr 20, 2021

    @bagder
    Member

    It seems totally reasonable to suspect that commit for having caused this regression. Unfortunately, that was a refactor that can't be reversed easily so the fix for this issue cannot be as easy as to revert that commit. This use case is a bit complicated and we clearly don't test this setup properly - as then we would've caught this in the CI before we merged this change.

    I don't know when I'll get time to look at this any closer.

  4. added a commit that references this issue on May 17, 2021
    9cb8360
  5. PhilETaylor commented on Jun 10, 2021

    @PhilETaylor

    Is this the related to the report here? https://gitlab.alpinelinux.org/alpine/aports/-/issues/12733

  6. bagder commented on Jun 10, 2021

    @bagder
    Member

    Hardly, this issue involved SFTP - and is already fixed.

  7. PhilETaylor commented on Jun 10, 2021

    @PhilETaylor

    No worries, proves that I was searching before posting a new issue though ha ha

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions