Remove checking for special characters in shells

This commit removes our logic that checks for special characters
in shell injection tests.  Really, all we care about is whether
format string characters are being used - if so we're probably
taking some kind of user input.  If not, it doesn't matter
whether we're calling something with special characters.

Change-Id: I7e6a8c45a25608e3a8ab8a7eca8d8f2de5dd9837
Closes-Bug: #1650393

bandit/plugins/injection_shell.py

@@ -15,33 +15,16 @@
 # under the License.
 
 import ast
-import re
 
 import bandit
 from bandit.core import test_properties as test
 
 
-def _has_special_characters(command):
-    # check if it contains any of the characters that may cause globing,
-    # multiple commands, subshell, or variable resolution
-    # glob: [ { * ?
-    # variable: $
-    # subshell: ` $
-    return bool(re.search(r'[{|\[;$\*\?`]', command))
-
-
 def _evaluate_shell_call(context):
     no_formatting = isinstance(context.node.args[0], ast.Str)
-    if no_formatting:
-        command = context.call_args[0]
-        no_special_chars = not _has_special_characters(command)
-    else:
-        no_special_chars = False
 
-    if no_formatting and no_special_chars:
+    if no_formatting:
         return bandit.LOW
-    elif no_formatting:
-        return bandit.MEDIUM
     else:
         return bandit.HIGH
 
@@ -204,15 +187,6 @@ def subprocess_popen_with_shell_equals_true(context, config):
                              'rewriting without shell',
                         lineno=context.get_lineno_for_call_arg('shell'),
                     )
-                elif sev == bandit.MEDIUM:
-                    return bandit.Issue(
-                        severity=bandit.MEDIUM,
-                        confidence=bandit.HIGH,
-                        text='call with shell=True contains special shell '
-                             'characters, consider moving extra logic into '
-                             'Python code',
-                        lineno=context.get_lineno_for_call_arg('shell'),
-                    )
                 else:
                     return bandit.Issue(
                         severity=bandit.HIGH,
@@ -464,14 +438,6 @@ def start_process_with_a_shell(context, config):
                          'Seems safe, but may be changed in the future, '
                          'consider rewriting without shell'
                 )
-            elif sev == bandit.MEDIUM:
-                return bandit.Issue(
-                    severity=bandit.MEDIUM,
-                    confidence=bandit.HIGH,
-                    text='Starting a process with a shell and special shell '
-                         'characters, consider moving extra logic into '
-                         'Python code'
-                )
             else:
                 return bandit.Issue(
                     severity=bandit.HIGH,

examples/subprocess_shell.py

@@ -29,3 +29,5 @@ subprocess.Popen('/bin/ls {}'.format('something'), shell=True)
 
 command = "/bin/ls" + unknown_function()
 subprocess.Popen(command, shell=True)
+
+subprocess.Popen('/bin/ls && cat /etc/passwd', shell=True)

tests/functional/test_functional.py

@@ -227,7 +227,7 @@ class FunctionalTests(testtools.TestCase):
 
     def test_os_popen(self):
         '''Test for `os.popen`.'''
-        expect = {'SEVERITY': {'LOW': 7, 'MEDIUM': 1, 'HIGH': 1},
+        expect = {'SEVERITY': {'LOW': 8, 'MEDIUM': 0, 'HIGH': 1},
                   'CONFIDENCE': {'HIGH': 9}}
         self.check_example('os-popen.py', expect)
 
@@ -256,7 +256,7 @@ class FunctionalTests(testtools.TestCase):
 
     def test_popen_wrappers(self):
         '''Test the `popen2` and `commands` modules.'''
-        expect = {'SEVERITY': {'MEDIUM': 7}, 'CONFIDENCE': {'HIGH': 7}}
+        expect = {'SEVERITY': {'LOW': 7}, 'CONFIDENCE': {'HIGH': 7}}
         self.check_example('popen_wrappers.py', expect)
 
     def test_random_module(self):
@@ -297,8 +297,8 @@ class FunctionalTests(testtools.TestCase):
     def test_subprocess_shell(self):
         '''Test for `subprocess.Popen` with `shell=True`.'''
         expect = {
-            'SEVERITY': {'HIGH': 3, 'MEDIUM': 2, 'LOW': 12},
-            'CONFIDENCE': {'HIGH': 16, 'LOW': 1}
+            'SEVERITY': {'HIGH': 3, 'MEDIUM': 1, 'LOW': 14},
+            'CONFIDENCE': {'HIGH': 17, 'LOW': 1}
         }
         self.check_example('subprocess_shell.py', expect)
 
@@ -318,7 +318,7 @@ class FunctionalTests(testtools.TestCase):
     def test_wildcard_injection(self):
         '''Test for wildcard injection in shell commands.'''
         expect = {
-            'SEVERITY': {'HIGH': 4, 'MEDIUM': 4, 'LOW': 6},
+            'SEVERITY': {'HIGH': 4, 'MEDIUM': 0, 'LOW': 10},
             'CONFIDENCE': {'MEDIUM': 5, 'HIGH': 9}
         }
         self.check_example('wildcard-injection.py', expect)