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

Crash with python3 when server sends debug messages #429

Closed
mjmaenpaa opened this issue Nov 6, 2014 · 5 comments
Closed

Crash with python3 when server sends debug messages #429

mjmaenpaa opened this issue Nov 6, 2014 · 5 comments

Comments

@mjmaenpaa
Copy link

@mjmaenpaa mjmaenpaa commented Nov 6, 2014

Paramiko crashes with python3 when server sends debug messages.

It seems that util.safe_string() hasn't been updated to handle bytes in Python3.
In Transport._parse_debug() msg is extracted from m with m.get_string() which in Python3 returns bytes-sequence. This message is given to safe_string(), which expects that message can be iterated over. However in Python3 iterating over bytes-sequence returns int instead of bytes and it can't be added to string implicitly.

Traceback:

  File "lib/python3.4/site-packages/paramiko/client.py", line 307, in connect
    look_for_keys, gss_auth, gss_kex, gss_deleg_creds, gss_host)
  File "lib/python3.4/site-packages/paramiko/client.py", line 433, in _auth
    allowed_types = self._transport.auth_publickey(username, pkey)
  File "lib/python3.4/site-packages/paramiko/transport.py", line 1232, in auth_publickey
    return self.auth_handler.wait_for_response(my_event)
  File "lib/python3.4/site-packages/paramiko/auth_handler.py", line 197, in wait_for_response
    raise e
  File "lib/python3.4/site-packages/paramiko/transport.py", line 1601, in run
    self._parse_debug(m)
  File "lib/python3.4/site-packages/paramiko/transport.py", line 2152, in _parse_debug
    self._log(DEBUG, 'Debug msg: ' + util.safe_string(msg))
  File "lib/python3.4/site-packages/paramiko/util.py", line 121, in safe_string
    out += c
TypeError: Can't convert 'int' object to str implicitly
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Nov 6, 2014

Can you confirm your Paramiko version? I recall we made a handful of changes in recent releases that might already account for this.

If you're recreating the issue on 1.15.1 (latest release), can you provide details on how to reproduce this so we can proceed with a fix? Thanks!

@mjmaenpaa
Copy link
Author

@mjmaenpaa mjmaenpaa commented Nov 7, 2014

I tested with 1.15.1 and it still crashes. Easiest way to reproduce is by slightly modifying demo_server.py to sent a debug message and running against it.

Following client side code should reproduce the issue when run against linked modified demo_server.py

import paramiko

ssh = paramiko.SSHClient()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
ssh.connect('localhost', port=2200, username='robey', password='foo')
shell = ssh.invoke_shell()
data = shell.recv(1024)
ssh.close()
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Nov 12, 2014

Thanks, can confirm reproduction of traceback (had to change the connect to 127.0.0.1 but that's a thing with Mac workstations re: IPv4 vs IPv6, iirc, and is orthogonal in any case.) Poking.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Nov 12, 2014

Heh this printing of debug messages is presently the only place safe_string is used and neither the calling or definition of safe_string have changed since 2004 apparently. Fun times.

bitprophet added a commit that referenced this issue Nov 12, 2014
bitprophet added a commit that referenced this issue Nov 12, 2014
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Nov 12, 2014

Think I got it, can no longer reproduce after this fix is in, and added a test besides. Will appear in the next set of bugfix releases. Thanks!

dkhapun pushed a commit to cyberx-labs/paramiko that referenced this issue Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants