Remove paramiko invoke_shell and fix example #377
Conversation
examples/paramiko_injection.py
Outdated
|
|
||
| # this is not safe | ||
| SSHClient.invoke_shell('something; bad; here\n') | ||
|
|
||
| client.invoke_shell('something; bad; here\n') |
There was a problem hiding this comment.
I think this is an interactive session and doesn't need an argument. But doesn't really matter for the test.
http://docs.paramiko.org/en/2.4/api/client.html#paramiko.client.SSHClient.invoke_shell
There was a problem hiding this comment.
Good catch. invoke_shell should have never been part of the check. I'll update the example and the plugin
Paramiko's invoke_shell function does not take a command argument even though the Bandit example implied that. It simply opens a stream for communicating with a shell. Therefore, it should not be flagged as part of the Bandit scan. The current example of paramiko command injection does not properly create an instance of the SSHClient before calling the functions on the client. Instead it's calling the functions statically which is not proper syntax. This patches updates the plugin and example. Bandit, however, is still functioning properly to detect the improper use of exec_command(). Fixes Issue #375 Signed-off-by: Eric Brown <browne@vmware.com>
ericwb
changed the title
|
Really, paramiko should be excluded from scanning. The whole point of paramiko is to be able to run arbitrary shell commands. How exactly is the programmer supposed to accomplish this without use of bandit should not bother marking something as invalid until it can recommend at least one valid replacement. Otherwise, it's just complaining "Don't do this" when in fact the programmer must do this. |
|
@mcandre Did you overcome this behaviour? |
I disagree with a caveat: yes, this is how paramiko is designed, but depending on what is passed in, one could get shell injection issues of some flavor (like shell=True with subprocess). B601 should be treated like a warning though and not an error, since it can’t really be worked around with the way the APIs are constructed today. I think this is worthy of an enhancement in bandit/paramiko, if the work isn’t too difficult to complete. |
Paramiko's invoke_shell function does not take a command argument
even though the Bandit example implied that. It simply opens a stream
for communicating with a shell. Therefore, it should not be flagged
as part of the Bandit scan.
The current example of paramiko command injection does not properly
create an instance of the SSHClient before calling the functions
on the client. Instead it's calling the functions statically which
is not proper syntax.
This patches updates the plugin and example. Bandit, however, is
still functioning properly to detect the improper use of exec_command().
Fixes Issue #375
Signed-off-by: Eric Brown browne@vmware.com