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

SFTP file downloads causing warnings #1933

Closed
Josh-LS opened this issue Aug 22, 2023 · 3 comments
Closed

SFTP file downloads causing warnings #1933

Josh-LS opened this issue Aug 22, 2023 · 3 comments

Comments

@Josh-LS
Copy link

Josh-LS commented Aug 22, 2023

Hello,

my current project is using phpseclib 1.0. I am trying to download a file from a SFTP server that only supports SFTP version 2.
The downloads were working fine for a while, until I started to get warnings like this:

Warning: unpack(): Type N: not enough input, need 4, have 0 in [...]/phpseclib/Net/SFTP.php on line 930
[...]
Net_SFTP->get( ???, ???, ???, ???, ??? ) - .../SFTPConnection.php:93
Net_SFTP->_logError( ???, ??? ) - .../SFTP.php:2727
[unpack](http://www.php.net/function.unpack) ( ???, ??? ) - .../SFTP.php:930

Debugging the code, I found that the _logError() function was called with a $response with a length of 4 characters and the default $status parameter of -1.

if ($status == -1) {
    if (strlen($response) < 4) {
        return;
    }
    extract(unpack('Nstatus', $this->_string_shift($response, 4)));
}

After the initial extraction of the response status bytes, the $status variable now contains the code for NET_SFTP_STATUS_EOF. $response is now an empty string, as the first and only 4 characters have been removed by the _string_shift() method.

Now the code checks the SFTP version and the length of the remaining response:

$error = $this->status_codes[$status];

if ($this->version > 2 || strlen($response) < 4) {
    extract(unpack('Nlength', $this->_string_shift($response, 4)));
    $this->sftp_errors[] = $error . ': ' . $this->_string_shift($response, $length);
} else {
    $this->sftp_errors[] = $error;
}

The current SFTP version is 2, but as the $response is empty the strlen($response) < 4 condition will still evaluate to true. The code then tries to extract the last 4 characters from the $response which will always have less than 4 characters at this point with this SFTP version, causing the above warnings.

I also checked other phpseclib versions. 2.0 looks similar to this, in 3.0 however the response length has been removed from the condition:

private function logError($response, $status = -1)
{
    if ($status == -1) {
        list($status) = Strings::unpackSSH2('N', $response);
    }

    $error = self::$status_codes[$status];

    if ($this->version > 2) {
        list($message) = Strings::unpackSSH2('s', $response);
        $this->sftp_errors[] = "$error: $message";
    } else {
        $this->sftp_errors[] = $error;
    }
}

Im not an expert on this and since the file downloads didnt cause warnings for a while, I am unsure if this is an issue with the phpseclib 1.0 SFTP implementation or if there could be another cause. The files are still properly downloaded despite the warnings fwiw.

@terrafrost
Copy link
Member

The previous SFTP client I had tested most of my v2/4/5/6 SFTP work against (Syncplify.me Server!) no longer seems to let you set the version of SFTP. Or at least if it does it's unclear to me how to do it.

I'll be looking at other SFTP clients (OpenSSH only supports SFTP v3) but, in the mean time... would you be willing to provide me with SFTP access to reproduce the problem?

If so you could email the connection info to terrafrost@php.net.

And if you can't... like I said, I'm looking into alternatives.

Thanks!

@terrafrost
Copy link
Member

I think 0f23be3 should fix this.

@Josh-LS
Copy link
Author

Josh-LS commented Sep 5, 2023

Unfortunately i can't give you access to the server i am using, but i applied the same fix on my end and it worked.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants