Bug 1489736

+++ This bug was initially created as a clone of Bug #1489733 +++

Description of the problem:

When doing SCP transfers using libssh2 client (e.g. curl), libssh2 generates a command to be sent in SSH_MSG_CHANNEL_REQUEST (exec request) based on path received as a user input, adds quotes around the path, and the scp command and its options. However the memory used for the command string was not initialized and uninitialized memory is sent over the secure channel. This may potentially leak some private information to the SCP/SSH remote server, and the unexpected bytes after the terminating quote 

There is an upstream fix for that in libssh2-1.8.0, libssh2 github commits: 

https://github.com/libssh2/libssh2/commit/b99204f2896b0cdafa3ecc0736f0252ce44c32c7#diff-692a9f59c318be98d982afb140caade9

and 

https://github.com/libssh2/libssh2/commit/1e7988cb0d8dae32148b04dd93e919a770599f30#diff-692a9f59c318be98d982afb140caade9

There is also an upstream fix improving the command length calculation, it seems: 

https://github.com/libssh2/libssh2/commit/3d3347c0625ce29b5581a0aa45e6e3be580769f1#diff-692a9f59c318be98d982afb140caade9

However, none of the above patches actually fixes the unexpected bytes in the command itself completely. The libssh2 library now appends a null terminating character at the end and some SCP/SSH server implementations (e.g. BrickFTP) fail on that and libssh2 clients cannot be used to do SCP transfers with them.

According to RFC 4521 definition of a string, null terminating characters are not used:

https://tools.ietf.org/html/rfc4251#section-9.5

I suggest a patch where the command length will not be increased by 1 after appending a terminating char and therefore the terminating null char will not be transferred. Please consider a necessity of the terminating null in general.


Version-Release number of selected component (if applicable):

Detected in libssh2-1.4.3-10.el7_2.1 (uninitialized memory)
Not fixed completely in libssh2-1.8.0-2.fc26 (null terminating char)

How reproducible:

The uninitialized memory and null terminating chars are visible when doing scp transfers using libssh2 built with --enable-debug, and a version of curl that uses the libssh2 debug function (commented out define in ssh.c).

Steps to Reproduce:

1. Build libssh2 with --enable-debug, build curl and libcurl with the line "#define CURL_LIBSSH2_DEBUG" uncommented in lib/ssh.c.
2. Use "curl --insecure -v -u user -T hello.txt scp://serverhostname/~/hello2.txt" to do an SCP upload.
3. Inspect the debug output.

Actual results:

Look for the exec request before encrypting:

[libssh2] 0.754346 Conn: starting request(exec) on channel 0/0, message=scp -t 'hello2.txt'
=> libssh2_transport_write plain (18 bytes)
0000: 62 00 00 00 00 00 00 00  04 65 78 65 63 01 00 00 : b........exec...
0010: 00 28                                            : .(
=> libssh2_transport_write plain2 (40 bytes)
0000: 73 63 70 20 2D 74 20 27  68 65 6C 6C 6F 32 2E 74 : scp -t 'hello2.t
0010: 78 74 27 00 6E 65 63 74  69 6F 6E 00 00 00 04 00 : xt'.nection.....
0020: 30 00 00 00 00 00 00 00                          : 0.......
[libssh2] 0.754375 Socket: Sent 100/100 bytes at 0xcc68c0

See the remains of a previous string appended after the scp -t 'hello2.txt' command?

Expected results:

The last byte of the command shall be the terminating single quote, not even a null byte.

Additional info:

To verify the upload/download functionality with e.g. BrickFTP service, you can setup a 30days trial account. BrickFTP SCP/SFTP server fails on SCP download/upload with curl/libssh2.

Fedora 26 and latest git have only the "null termination char" part of this bug.