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

sftp segfault with macOS curl 7.69.0 #5041

Closed
xquery opened this issue Mar 5, 2020 · 5 comments
Closed

sftp segfault with macOS curl 7.69.0 #5041

xquery opened this issue Mar 5, 2020 · 5 comments
Assignees
Labels

Comments

@xquery
Copy link
Member

@xquery xquery commented Mar 5, 2020

I did this

usr/local/bin/curl -u : sftp://test.rebex.net:22 -l --verbose

or

/usr/local/bin/curl -u : sftp://itcsubmit.wustl.edu -l

and got a segfault

I expected the following

not to segfault ;)

curl/libcurl version

curl 7.69.0-DEV (Darwin) libcurl/7.69.0-DEV OpenSSL/1.0.2s zlib/1.2.11 libssh2/1.9.0_DEV nghttp2/1.26.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM SSL UnixSockets

operating system

macOS Mojave

@bagder
Copy link
Member

@bagder bagder commented Mar 5, 2020

Program received signal SIGSEGV, Segmentation fault.
ssh_force_knownhost_key_type (conn=0x55555574fae8) at vssh/libssh2.c:697
697           if(store->name[0] == '[') {
(gdb) p store
$1 = (struct libssh2_knownhost *) 0x5555557aad80
(gdb) p store->name
$2 = 0x0

The struct in question is documented in libssh2.h:

struct libssh2_knownhost {
    unsigned int magic;  /* magic stored by the library */
    void *node; /* handle to the internal representation of this host */
    char *name; /* this is NULL if no plain text host name exists */
    char *key;  /* key in base64/printable format */
    int typemask;
};

and it specifically says it can be NULL so our code is just not correct!

@bagder
Copy link
Member

@bagder bagder commented Mar 7, 2020

(This is a regression from #4747 by @SantinoKeupp)

@emilengler
Copy link
Contributor

@emilengler emilengler commented Mar 8, 2020

Can reproduce on 0a04dc4

curl 7.69.1-DEV (x86_64-pc-linux-gnu) libcurl/7.69.1-DEV OpenSSL/1.1.1d zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.36.0 librtmp/2.3
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS HTTP2 HTTPS-proxy IDN IPv6 Largefile libz NTLM NTLM_WB PSL SSL TLS-SRP UnixSockets

On 7.64.0

curl 7.64.0 (x86_64-pc-linux-gnu) libcurl/7.64.0 OpenSSL/1.1.1d zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.36.0 librtmp/2.3
Release-Date: 2019-02-06
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy PSL 

I get the error that the SSL cert is bad. If I pass -K it also segfault but different

@xquery
Copy link
Member Author

@xquery xquery commented Mar 8, 2020

thx - the fix is straightforward eg. check for null in vssh/libssh2.c:697 (which as @bagder pointed out was added in #4747 and caused the regression)... building out the test is most of the work eg. I also want to include a few representative known_hosts files (with and without corrupted entries) - will have PR ready for review tomorrow.

@emilengler
Copy link
Contributor

@emilengler emilengler commented Mar 8, 2020

Thanks. Feel free to ping me for review!

xquery added a commit to xquery/curl that referenced this issue Mar 8, 2020
This fix adds a defensive check for the case where the
char *name in struct libssh2_knownhost is NULL

Fixes curl#5041
@bagder bagder closed this in e96fe70 Mar 9, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.