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

PHP Warning: unpack(): Type C: not enough input, need 1, have 0 in SSH2.php on line 3015 #967

Closed
rvermaat opened this issue Apr 8, 2016 · 8 comments

Comments

@rvermaat
Copy link

rvermaat commented Apr 8, 2016

Hi,

I have an application that connects to a ssh server an get lots of data using the interactive shell (write/read). This application uses telnet and thats works fine. But i wanted to convert it to ssh, to be more secure.

Connecting to the server was fine. getting small amound of data also worked fine.
But when i gathered lots of bulk data it allways went wrong and get error like.

Warning: extract() expects parameter 1 to be array, boolean given in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3015
PHP Notice:  Undefined variable: packet_length in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3021

Notice: Undefined variable: packet_length in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3021
PHP Notice:  Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027

Notice: Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027
PHP Notice:  Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320

Notice: Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320
PHP Notice:  Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027

Notice: Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027
PHP Notice:  Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320

Notice: Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320
PHP Notice:  Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027

Notice: Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027
PHP Notice:  Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320

Notice: Connection closed by server in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3320
PHP Notice:  Invalid size in /var/www/ims/ssp/vendor/Net/SSH2.php on line 3027

After a lot of debugging and reading PHP Bug #52602 I changed the fread function in SSH2.php to stream_get_contents. this fixed my problem

@terrafrost
Copy link
Member

Seems like stream_socket_recvfrom might be better?

That said, neither stream_socket_recvfrom or stream_get_contents could be used in the 1.0 branch to preserve BC (that branch is compatible with PHP 4.3+).

What branch are you using anyway? You don't appear to be using 1.0.1 or 2.0.1 as line 3021 doesn't have packet_length in either of those tags.

@rvermaat
Copy link
Author

Just tried stream_socket_recvfrom ; did not work for me. I'm back with stream_get_contents. This seems to work perfectly for me.

I understand that using stream_get_contents will break BC when using v1.0.

I now use the 1.0 branch. When i did some debugging i put some comments line etc in the code. Thats why the line numbers do not match..

I also tried the 2.0 branch , but that failed on the random_bytes() function (i'm using php 5.4).
Is the 2.0 branch production ready ?

@terrafrost
Copy link
Member

I also tried the 2.0 branch , but that failed on the random_bytes() function (i'm using php 5.4).
Is the 2.0 branch production ready ?

You should be using tagged releases. eg. 2.0.0 or 2.0.1. The 2.0 branch, will never be production ready, per-say, because it's the tagged releases you should be using. I may, unintentionally, make a commit that may break the 2.0 branch, from time to time, and that's not a huge problem as you shouldn't be using that branch anyway. You should use the tagged releases. And if a release is botched a new release will be made.

The specific issue you encountered with the 2.0 branch is probably because (1) you're not using composer and (2) the 2.0 branch recently added https://github.com/paragonie/random_compat as a dependency. If you just download the 2.0 branch it won't work. The 2.0 branch requires a PSR-4 compliant auto-loader so when I release 2.0.2 and if I were to create a zip file for it it would need to have random_compat included within it's directory structure such that a strictly PSR-4 compliant auto-loader would works. If you use composer it's not an issue because composer will manage all the dependencies for you. But downloading https://github.com/phpseclib/phpseclib/archive/2.0.2.zip... I don't know if that should even be considered an officially supported download source. I mean, I guess I could add random_compat as a submodule for the 2.0 branch for people trying to download https://github.com/phpseclib/phpseclib/archive/2.0.2.zip but idk.

@terrafrost
Copy link
Member

BTW my plan for the master branch, that'll eventually be 3.0.x, is to make that one require composer. So whereas 2.0.x can be made to work without composer without too much effort and be BC with past 2.0.x versions 3.0.x... not so much. For example composer let's you do more than just PSR-4 autoloading so with 3.0.x if you're not using composer you might have to add extra include's to every file using phpseclib when you upgrade phpseclib to a new version whereas with 2.0.x you wouldn't have to.

@rvermaat
Copy link
Author

I now use the tagged 2.0.1 branch. Added the https://github.com/paragonie/random_compat to my project and no more fatal error on the random_bytes() function. Still have the original problem and change fread() into stream_get_contents(). And also with 2.0.1 this works for me.

You are correct. I don't use composer. I do use the autoloader from composer to load all of my own and vendor classes. I do this because most of the machines I run the applications on do not have access to the internet.

I hope you reconsider that the 3.0.x release requires composer. A manual install with a correct autoloader (the one composer uses) and a list of dependencies is appreciated .

@terrafrost
Copy link
Member

Still have the original problem and change fread() into stream_get_contents()

Yah - I still need to take action on that.

W.r.t. composer... what I'm tentatively thinking is that I could have a downloadable version that has the autoload file that composer generates included within it. Like Composer has a PSR-4 autoloader but it also does, for example, files autoloading too. So when you do composer update or composer install it creates / modifies vendor/autoload.php:

<?php

// autoload.php @generated by Composer

require_once __DIR__ . '/composer' . '/autoload_real.php';

return ComposerAutoloaderInit75fd5e89db521a456c50cc9b84a6cefb::getLoader();

autoload_real.php includes (among other files) autoload_files.php which looks like this:

<?php

// autoload_files.php @generated by Composer

$vendorDir = dirname(dirname(__FILE__));
$baseDir = dirname($vendorDir);

return array(
    '5255c38a0faeba867671b61dfda6d864' => $vendorDir . '/paragonie/random_compat/lib/random.php',
    'decc78cc4436b1292c6c0d151b19445c' => $vendorDir . '/phpseclib/phpseclib/phpseclib/bootstrap.php',
);

So Composer has it's own autoloaders but if you do composer update / composer install it creates a custom auto-loader for your app based on your composer.json.

Other Composer packages might result in aneven bigger array being returned by autoload_files.php as well.

@terrafrost
Copy link
Member

Here's a version of the 1.0 branch that uses stream_get_contents():

https://github.com/terrafrost/phpseclib/tree/stream-get-contents-1.0

I also replaced fgets() with stream_get_line() but that one wasn't a drop in replacement - I had to change the code in other ways.

One thing I want to look into: maybe I could do this in the 1.0 branch and use PHP_Compat to emulate stream_get_contents() and stream_get_line()? But idk if PHP_Compat is even maintained anymore. Maybe I'll just do it for 2.0+ or maybe just master and beyond idk. Stuff to think about.

@terrafrost
Copy link
Member

This is in the 2.0 and master branches. I decided not to do it for the 1.0 branch because I'm not sure stream_get_line() can really be emulated with a PHP_Compat style function. I guess I could do an if check on PHP_VERSION for stream_get_line() but then what about stream_get_contents()?

Currently, 4x PHP_Compat functions are needed for the 1.0 branch to work on PHP4:

https://github.com/phpseclib/phpseclib/tree/0.2.2/phpseclib/PHP/Compat/Function
https://github.com/pear/PHP_Compat/blob/master/PHP/Compat/Function/clone.php

The problem is that it doesn't appear that PHP_Compat is maintained anymore. So at that point it'd make sense for phpseclib to maintain it's own copy of PHP_Compat, at, for example, https://github.com/phpseclib/php_compat or some such. But on top of that I'd also have to write my own stream_get_contents() function since there isn't one in PHP_Compat. And, honestly, I don't know that I care to go to all that effort.

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