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

Jump to bottom

sftp segfault with macOS curl 7.69.0 #5041

Closed
xquery opened this issue Mar 5, 2020 · 5 comments · Fixed by sthagen/curl-curl#129
Closed

sftp segfault with macOS curl 7.69.0 #5041

xquery opened this issue Mar 5, 2020 · 5 comments · Fixed by sthagen/curl-curl#129
Assignees
@xquery
Labels
crash SCP/SFTP

Comments

@xquery
Copy link
Member

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 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 commented Mar 7, 2020

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

@emilengler
Copy link
Contributor

emilengler commented Mar 8, 2020 •
edited

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 commented Mar 8, 2020 •
edited

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

Thanks. Feel free to ping me for review!

xquery added a commit to xquery/curl that referenced this issue Mar 8, 2020
@xquery
sftp: fix segfault regression introduced by curl#4747
c5c5ffe 
This fix adds a defensive check for the case where the
char *name in struct libssh2_knownhost is NULL

Fixes curl#5041
@xquery xquery mentioned this issue Mar 8, 2020
Closed
@bagder bagder closed this as completed in e96fe70 Mar 9, 2020
@sthagen sthagen mentioned this issue Mar 9, 2020
Merged
@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.
Assignees

@xquery xquery

Labels
crash SCP/SFTP
Milestone
No milestone
3 participants
@bagder @xquery @emilengler