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

Security issue - throwing username and password in stacktrace #1118

Open
gandra opened this issue Apr 18, 2017 · 4 comments
Open

Security issue - throwing username and password in stacktrace #1118

gandra opened this issue Apr 18, 2017 · 4 comments

Comments

@gandra
Copy link

gandra commented Apr 18, 2017

I am playing with thephpleague flysystem and under the hood it use phpseclib. When there is a connection problem in the stack trace are thrown credentials:

in SSH2.php line 1011
at HandleExceptions->handleError(1024, 'Cannot connect to my-host.com:22. Error 0. php_network_getaddresses: getaddrinfo failed: Name or service not known', '/home/vagrant/Code/my-project/vendor/phpseclib/phpseclib/phpseclib/Net/SSH2.php', 1011, array('start' => 1492543756.398536, 'errno' => 0, 'errstr' => 'php_network_getaddresses: getaddrinfo failed: Name or service not known', 'host' => 'my-host.com:22'))
at user_error('Cannot connect to my-host.com:22. Error 0. php_network_getaddresses: getaddrinfo failed: Name or service not known') in SSH2.php line 1011
at SSH2->_connect() in SSH2.php line 1888
at SSH2->_login('username', 'secret')
at call_user_func_array(array(object(SFTP), '_login'), array('username', 'secret')) in SFTP.php line 390
at SFTP->login('username', 'secret') in SftpAdapter.php line 193
...

I consider it as a security hole. Username and password should not be thrown in the stacktrace.
At least by default. If for some reason you still prefer to throw password in the stacktrace it should be enabled by explicit configuration. By default password should not be thrown as part of a stack trace.

@GrahamCampbell
Copy link
Member

Not a security bug imo.

By default password should not be thrown as part of a stack trace.

Not how stack traces work.

@gandra
Copy link
Author

gandra commented Apr 18, 2017

@GrahamCampbell ,
I have seen my password in the stacktrace. I think this should not happen. Password should not appear in the stacktrace.

@GrahamCampbell
Copy link
Member

Password should not appear in the stacktrace.

Right, but my point is, how do you propose we do this...

@terrafrost
Copy link
Member

I guess thephpleague takes errors and turns them into exceptions that are thrown. So in that case you could simply put the login() method around a try / catch block and catch all Exceptions.

Sample code:

<?php
function login($username, $password) {
    throw new Exception();
}

login('user', 'pass');

Oh noes, you can see the user / pass in the stack trace.

<?php
function login($username, $password) {
    throw new Exception();
}

try {
    login('user', 'pass');
} catch (Exception $e) {}

Yay, no full stack trace.

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

3 participants