mirror of
https://github.com/libssh2/libssh2.git
synced 2026-10-24 03:48:58 +03:00
sftp: free response buffer on sftp_read() protocol-error paths
sftp_read takes ownership of the FXP_DATA response buffer that sftp_packet_requirev unlinks from the packet brigade, but three protocol-error branches in the SSH_FXP_DATA arm return without freeing it: an unexpected chunk offset, a declared data length past the end of the packet body, and a chunk larger than the one we requested. A server can trigger any of these on a read and leak the whole response buffer each time it happens. Free data before those returns, the way the neighbouring SSH_FXP_STATUS arm already does. Closes #2537
This commit is contained in:
committed by
Viktor Szakats
parent
86adba149f
commit
cdbda3cd8d
12
src/sftp.c
12
src/sftp.c
@@ -1655,26 +1655,32 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE *handle,
|
||||
}
|
||||
|
||||
case SSH_FXP_DATA:
|
||||
if(chunk->offset != filep->offset)
|
||||
if(chunk->offset != filep->offset) {
|
||||
/* This could happen if the server returns less bytes than
|
||||
requested, which should not happen for normal files.
|
||||
See:
|
||||
https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-02#section-6.4
|
||||
*/
|
||||
SSH2_FREE(session, data);
|
||||
return ssh2_err(session, LIBSSH2_ERROR_SFTP_PROTOCOL,
|
||||
"Read Packet At Unexpected Offset");
|
||||
}
|
||||
|
||||
rc32 = ssh2_ntohu32(data + 5);
|
||||
if(rc32 > (data_len - 9))
|
||||
if(rc32 > (data_len - 9)) {
|
||||
SSH2_FREE(session, data);
|
||||
return ssh2_err(session, LIBSSH2_ERROR_SFTP_PROTOCOL,
|
||||
"SFTP Protocol badness");
|
||||
}
|
||||
|
||||
if(rc32 > chunk->len)
|
||||
if(rc32 > chunk->len) {
|
||||
/* A chunk larger than we requested was returned to us.
|
||||
This is a protocol violation and we do not know how to
|
||||
deal with it. Bail out! */
|
||||
SSH2_FREE(session, data);
|
||||
return ssh2_err(session, LIBSSH2_ERROR_SFTP_PROTOCOL,
|
||||
"FXP_READ response too big");
|
||||
}
|
||||
|
||||
if(rc32 != chunk->len)
|
||||
/* a short read does not imply end of file, but we must
|
||||
|
||||
Reference in New Issue
Block a user