On 6/14/2016 1:41 AM, ????? _ wrote:
> I met a crash issue when using libcurl to download file from an SFTP server.
>
> The server software is Bitvise SSH server 6.47. Client is running on CentOS 7.
>
> The issue happens when I accidentally set a user's real root path to an non-exsit directory.
>
> I compiled a debug .so with version 7.49.1 and tried a little dig in.
>
> Below is part of the trace
>
> (gdb) bt
> #0 0x00007fca58a668f1 in __strlen_sse2_pminub () from /lib64/libc.so.6
> #1 0x00007fca5abed10f in ssh_getworkingpath (conn=0x7fc9580131d8, homedir=0x0, path=0x7fc958013b98) at ssh.c:441
> #2 0x00007fca5abeea81 in ssh_statemach_act (conn=0x7fc9580131d8, block=0x7fc9be7fb223) at ssh.c:1182
> #3 0x00007fca5abf3377 in ssh_multi_statemach (conn=0x7fc9580131d8, done=0x7fc9be7fb343) at ssh.c:2810
> #4 0x00007fca5abf3c31 in sftp_perform (conn=0x7fc9580131d8, connected=0x7fc9be7fb297, dophase_done=0x7fc9be7fb343) at ssh.c:3185
> #5 0x00007fca5abf3942 in ssh_do (conn=0x7fc9580131d8, done=0x7fc9be7fb343) at ssh.c:3041
> #6 0x00007fca5abe0f99 in multi_do (connp=0x7fc9580009a8, done=0x7fc9be7fb343) at multi.c:1230
> #7 0x00007fca5abe1c66 in multi_runsingle (multi=0x7fc958009ad8, now=..., data=0x7fc958000998) at multi.c:1627
> #8 0x00007fca5abe2bcc in curl_multi_perform (multi_handle=0x7fc958009ad8, running_handles=0x7fc9be7fb4fc) at multi.c:2122
> #9 0x00007fca5abd71c0 in easy_transfer (multi=0x7fc958009ad8) at easy.c:726
> #10 0x00007fca5abd739d in easy_perform (data=0x7fc958000998, events=false) at easy.c:813
> #11 0x00007fca5abd73e7 in curl_easy_perform (easy=0x7fc958000998) at easy.c:832
>
> Line 441 of ssh.c
> size_t homelen = strlen(homedir);
>
> homedir is a NULL pointer here.
>
>
> Tracking back it seems the problem is at state SSH_SFTP_REALPATH (ssh.c line 1139 -1179)
>
> case SSH_SFTP_REALPATH:
> {
> char tempHome[PATH_MAX];
>
> /*
> * Get the "home" directory
> */
> rc = sftp_libssh2_realpath(sshc->sftp_session, ".",
> tempHome, PATH_MAX-1);
> if(rc == LIBSSH2_ERROR_EAGAIN) {
> break;
> }
> else if(rc > 0) {
> /* It seems that this string is not always NULL terminated */
> tempHome[rc] = '\0';
> sshc->homedir = strdup(tempHome);
> if(!sshc->homedir) {
> state(conn, SSH_SFTP_CLOSE);
> sshc->actualcode = CURLE_OUT_OF_MEMORY;
> break;
> }
> conn->data->state.most_recent_ftp_entrypath = sshc->homedir;
> }
> else {
> /* Return the error type */
> err = sftp_libssh2_last_error(sshc->sftp_session);
> result = sftp_libssh2_error_to_CURLE(err);
> sshc->actualcode = result?result:CURLE_SSH;
>
> DEBUGF(infof(data, "error = %d makes libcurl = %d\n",
> err, (int)result));
> state(conn, SSH_STOP);
> break;
> }
> }
>
>
>
> In this case , the "rc" returned by libssh2 is 0 ( I dumped the values with gdb),
> it runs into the "else" branch, while "err" and "result" are 0 too.
>
> This leaves sshc->homedir not set but return a result of CURLE_OK.
> The state machine will continue working and finally to access the NULL homedir.
>
> Could someone help to confirm if this is an issue?

sftp_libssh2_realpath is a function-like macro that maps to
libssh2_sftp_symlink_ex [1] so the call is:

rc = libssh2_sftp_symlink_ex(sshc->sftp_session,
                              ".", strlen("."),
                              tempHome, PATH_MAX-1,
                              LIBSSH2_SFTP_REALPATH);

The documentation allows for the case of an empty homedir since it does
not specifically address it. So I think in this case we must assume it
is not an error. Perhaps we change the success case from rc > 0 to rc >=
0, and make it an empty path if that doesn't break something somewhere
else. Or make it /. I don't know what you can do when your root path is
invalid in Bitvise, I haven't tried that.

[1]: https://www.libssh2.org/libssh2_sftp_symlink_ex.html

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-06-14