Skip to content

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

Merge request reports

Loading