Repository: PyCQA/bandit
Pull request #377 (2018-08-27 18:06:15)

 Remove paramiko invoke_shell and fix example
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>

mcandre: 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 `exec_command`? Can you recommend a sanitizer for arbitrary shell commands? Would bandit recognize when an "approved" sanitizer is applied to the commands?

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.

Patch:
diff --git a/bandit/plugins/injection_paramiko.py b/bandit/plugins/injection_paramiko.py
index a87e13b..0f2008f 100644
--- a/bandit/plugins/injection_paramiko.py
+++ b/bandit/plugins/injection_paramiko.py
@@ -23,9 +23,8 @@ Paramiko is a Python library designed to work with the SSH2 protocol for secure
 (encrypted and authenticated) connections to remote machines. It is intended to
 run commands on a remote host. These commands are run within a shell on the
 target and are thus vulnerable to various shell injection attacks. Bandit
-reports a MEDIUM issue when it detects the use of Paramiko's "exec_command" or
-"invoke_shell" methods advising the user to check inputs are correctly
-sanitized.
+reports a MEDIUM issue when it detects the use of Paramiko's "exec_command"
+method advising the user to check inputs are correctly sanitized.
 
 :Example:
 
@@ -36,17 +35,9 @@ sanitized.
        Severity: Medium   Confidence: Medium
        Location: ./examples/paramiko_injection.py:4
     3    # this is not safe
-    4    paramiko.exec_command('something; reallly; unsafe')
+    4    paramiko.exec_command('something; really; unsafe')
     5
 
-    >> Issue: Possible shell injection via Paramiko call, check inputs are
-       properly sanitized.
-       Severity: Medium   Confidence: Medium
-       Location: ./examples/paramiko_injection.py:10
-    9    # this is not safe
-    10   SSHClient.invoke_shell('something; bad; here\n')
-    11
-
 .. seealso::
 
  - https://security.openstack.org
@@ -68,7 +59,7 @@ def paramiko_calls(context):
                   'are properly sanitized.')
     for module in ['paramiko']:
         if context.is_module_imported_like(module):
-            if context.call_function_name in ['exec_command', 'invoke_shell']:
+            if context.call_function_name in ['exec_command']:
                 return bandit.Issue(severity=bandit.MEDIUM,
                                     confidence=bandit.MEDIUM,
                                     text=issue_text)
diff --git a/examples/paramiko_injection.py b/examples/paramiko_injection.py
index 6d30322..abce4f8 100644
--- a/examples/paramiko_injection.py
+++ b/examples/paramiko_injection.py
@@ -1,11 +1,10 @@
 import paramiko
 
-# this is not safe
-paramiko.exec_command('something; really; unsafe')
 
-# this is safe
-paramiko.connect('somehost')
+client = paramiko.client.SSHClient()
 
 # this is not safe
-SSHClient.invoke_shell('something; bad; here\n')
+client.exec_command('something; really; unsafe')
 
+# this is safe
+client.connect('somehost')


Test patch:
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index 60010a5..335bd8b 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -612,8 +612,8 @@ class FunctionalTests(testtools.TestCase):
     def test_paramiko_injection(self):
         '''Test paramiko command execution.'''
         expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 2, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 2, 'HIGH': 0}
+            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 1, 'HIGH': 0},
+            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 1, 'HIGH': 0}
         }
         self.check_example('paramiko_injection.py', expect)