Bugfix sftp_packet_read potentially getting stuck in an infinite loop
Hi libssh devs! I've run into a bug where sftp_packet_read gets stuck in an infinite loop. Hence submitting this patch to fix it. Please help me review and get it merged.
Explaination of the bug
sftp_packet_read
keeps calling ssh_channel_read
and won't exist unless hitting an EOF or polling 4 bytes out.
do {
int s;
// read from channel until 4 bytes have been read or an error occurs
- s = ssh_channel_read(sftp->channel, tmpbuf + nread, 4 - nread, 0);
if (s < 0) {
goto error;
- } else if (s == 0) {
is_eof = ssh_channel_is_eof(sftp->channel);
if (is_eof) {
ssh_set_error(sftp->session,
SSH_FATAL,
"Received EOF while reading sftp packet size");
sftp_set_error(sftp, SSH_FX_EOF);
goto error;
}
} else {
nread += s;
}
} while (nread < 4);
But ssh_channel_read
could return 0 when no data is available in blocking mode. In the case where ssh_channel_read
always returns 0 after hitting a timeout, ssh_channel_is_eof(sftp->channel)
would return false, causing a deadloop here.
I've also added a regression test, torture_sftp_packet_read
to demonstrate this behaviour.
Checklist
-
Commits have Signed-off-by:
with name/author being identical to the commit author -
Code modified for feature -
Test suite updated with functionality tests -
Test suite updated with negative tests -
Documentation updated
Reviewer's checklist:
-
Any issues marked for closing are addressed -
There is a test suite reasonably covering new functionality or modifications -
Function naming, parameters, return values, types, etc., are consistent and according to CONTRIBUTING.md -
This feature/change has adequate documentation added -
No obvious mistakes in the code