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

libssh2_sftp_read() with maxlen of 8192 causes memory corruption #50

Closed
Corillian opened this issue Oct 2, 2015 · 4 comments

Comments

Projects
None yet
4 participants
@Corillian
Copy link

commented Oct 2, 2015

When calling libssh2_sftp_read() with a buffer size of 4096 my application works as expected. If I do nothing other than change the buffer size to 8192 the application begins crashing due to memory corruption within libssh2. I am using a blocking socket with libssh2 set to blocking mode. Problems start occurring when libssh2 arbitrarily skips ahead within the file. Sometimes it does it with the first read and sometimes it doesn't do it until after multiple successful reads:

NOTE: Requested size is always 8192

GetBuffer() [New] About to read from requested file position 0 and actual file position 0
GetBuffer() [New] Read completed: bytes read = 2768, totalByteRead = 2768, new actual file position = 10960
GetBuffer() [New] About to read from requested file position 0 and actual file position 10960
GetBuffer() [New] Read completed: bytes read = 5424, totalByteRead = 8192, new actual file position = 16384

If you look at the log output above you can see that the libssh2 file pointer position started at 0. 8192 bytes were requested, 2768 were returned (it's always this number once problems begin), and the new file pointer after read has somehow jumped to 10960.

I compiled libssh2 and all of its dependencies as 64-bit DLL's using Visual Studio 2013. It's linking to OpenSSL 1.0.2d and zlib 1.2.8. All of my testing was done with Windows 7 x64. My sequence of actions is:

  1. I set function callbacks with OpenSSL for creating and destroying locks
  2. Initialize winsock and libssh2
  3. Create a (blocking) socket
  4. Do the ssh handshake using a private key in memory
  5. Create the sftp session and open a file. When the sftp session is created I set function pointers to a custom set of memory allocation functions that just call malloc(), realloc(), and free()
  6. Begin reading from the file. Every call to libssh2_sftp_read() is preceded by libssh2_sftp_seek64() to explicitly set the file pointer

I can't provide a callstack because the memory corruption appears to be nuking it. All I get is an access violation at msvcr120.dll!000007fedea9c3f9() Unknown. If there's any additional information I can provide please let me know.

@sbredahl

This comment has been minimized.

Copy link
Contributor

commented Oct 7, 2015

I can confirm that some weird things happen if the buffer maxlen is much larger than 4k (7k works for me also, but 8k is too large). I end up in an infinite loop where libssh2_sftp_read() keeps returning 0 (in blocking mode).

I don't get any access violation however (also Win7 x64).

@sbredahl

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2015

I have investigated this issue a little further (using example-sftp) on both of my libssh2 builds (Ubuntu 14.10 and Windows 7) at work.

Corrupted downloads happen (in both builds) if buffer_maxlen * 4 > MAX_SFTP_READ_SIZE. So with the current MAX_SFTP_READ_SIZE=30000, problems occur if buffer_maxlen>7500.

Edit: It is actually the "max_read_ahead" value in sftp.c that causes the issue. It is set to 4*buffer_size but if it exceeds MAX_SFTP_READ_SIZE things break.

As a workaround we could use max_read_ahead = MIN(MAX_SFTP_READ_SIZE, 4*buffer_size) until someone figures out the right solution here...

jakob added a commit to jakob/libssh2 that referenced this issue Jan 13, 2016

sftp.c: stop reading when buffer is full
Since we can only store data from a single chunk in filep,
we have to stop receiving data as soon as we have to stop
receiving data as soon as the buffer is full.

This adresses the following bug report:
libssh2#50
@jakob

This comment has been minimized.

Copy link
Contributor

commented Jan 15, 2016

@Corillian Could you check if my branch https://github.com/jakob/libssh2/tree/sftp-read-fix (see pull request #75) fixes your issues?

jakob added a commit to jakob/libssh2 that referenced this issue Feb 4, 2016

sftp.c: stop reading when buffer is full
Since we can only store data from a single chunk in filep,
we have to stop receiving data as soon as the buffer is full.

This adresses the following bug report:
libssh2#50

bagder added a commit that referenced this issue Feb 11, 2016

sftp.c: stop reading when buffer is full
Since we can only store data from a single chunk in filep,
we have to stop receiving data as soon as the buffer is full.

This adresses the following bug report:
#50
@bagder

This comment has been minimized.

Copy link
Member

commented Feb 11, 2016

That fix is now merged into master and we believe this bug is fixed. Please provide details if not.

@bagder bagder closed this Feb 11, 2016

kbulgrien pushed a commit to kbulgrien/libssh2-1.2.4-sco3.2v5.0.7 that referenced this issue Dec 14, 2018

sftp.c: stop reading when buffer is full
Since we can only store data from a single chunk in filep,
we have to stop receiving data as soon as the buffer is full.

This adresses the following bug report:
libssh2#50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.