Merge branch 'master' into nosec

bandit/formatters/custom.py

@@ -13,21 +13,25 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-r"""
+"""
 ================
 Custom Formatter
 ================
 
 This formatter outputs the issues in custom machine-readable format.
 
-default template: {abspath}:{line}: {test_id}[bandit]: {severity}: {msg}
+default template: ``{abspath}:{line}: {test_id}[bandit]: {severity}: {msg}``
 
 :Example:
 
-/usr/lib/python3.6/site-packages/openlp/core/utils/__init__.py: \
+.. code-block:: none
+
+    /usr/lib/python3.6/site-packages/openlp/core/utils/__init__.py:\
 405: B310[bandit]: MEDIUM: Audit url open for permitted schemes. \
 Allowing use of file:/ or custom schemes is often unexpected.
 
+.. versionadded:: 1.5.0
+
 """
 
 import logging

bandit/formatters/screen.py

@@ -48,6 +48,17 @@
 from bandit.core import docs_utils
 from bandit.core import test_properties
 
+# This fixes terminal colors not displaying properly on Windows systems.
+# Colorama will intercept any ANSI escape codes and convert them to the
+# proper Windows console API calls to change text color.
+if sys.platform.startswith('win32'):
+    try:
+        import colorama
+    except ImportError:
+        pass
+    else:
+        colorama.init()
+
 LOG = logging.getLogger(__name__)
 
 COLOR = {

bandit/plugins/injection_paramiko.py

@@ -23,9 +23,8 @@
 (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 @@
        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)

bandit/plugins/injection_sql.py

@@ -28,6 +28,7 @@
  - "SELECT thing FROM " + tab
  - "SELECT " + val + " FROM " + tab + ...
  - "SELECT {} FROM derp;".format(var)
+ - f"SELECT foo FROM bar WHERE id = {product}"
 
 Unless care is taken to sanitize and control the input data when building such
 SQL statement strings, an injection attack becomes possible. If strings of this
@@ -93,6 +94,9 @@ def _evaluate_ast(node):
         statement = node.s
         # Hierarchy for "".format() is Wrapper -> Call -> Attribute -> Str
         wrapper = node.parent.parent.parent
+    elif isinstance(node.parent, ast.JoinedStr):
+        statement = node.s
+        wrapper = node.parent.parent
 
     if isinstance(wrapper, ast.Call):  # wrapped in "execute" call?
         names = ['execute', 'executemany']

bandit/plugins/yaml_load.py

@@ -57,15 +57,21 @@
 def yaml_load(context):
     imported = context.is_module_imported_exact('yaml')
     qualname = context.call_function_name_qual
-    if imported and isinstance(qualname, str):
-        qualname_list = qualname.split('.')
-        func = qualname_list[-1]
-        if 'yaml' in qualname_list and func == 'load':
-            if not context.check_call_arg_value('Loader', 'SafeLoader'):
-                return bandit.Issue(
-                    severity=bandit.MEDIUM,
-                    confidence=bandit.HIGH,
-                    text="Use of unsafe yaml load. Allows instantiation of"
-                         " arbitrary objects. Consider yaml.safe_load().",
-                    lineno=context.node.lineno,
-                )
+    if not imported and isinstance(qualname, str):
+        return
+
+    qualname_list = qualname.split('.')
+    func = qualname_list[-1]
+    if all([
+            'yaml' in qualname_list,
+            func == 'load',
+            not context.check_call_arg_value('Loader', 'SafeLoader'),
+            not context.check_call_arg_value('Loader', 'CSafeLoader'),
+    ]):
+        return bandit.Issue(
+            severity=bandit.MEDIUM,
+            confidence=bandit.HIGH,
+            text="Use of unsafe yaml load. Allows instantiation of"
+                 " arbitrary objects. Consider yaml.safe_load().",
+            lineno=context.node.lineno,
+        )

doc/source/formatters/custom.rst

@@ -0,0 +1,5 @@
+------
+custom
+------
+
+.. automodule:: bandit.formatters.custom

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')

examples/sql_statements-py36.py

@@ -0,0 +1,41 @@
+import sqlalchemy
+
+# bad
+query = "SELECT * FROM foo WHERE id = '%s'" % identifier
+query = "INSERT INTO foo VALUES ('a', 'b', '%s')" % value
+query = "DELETE FROM foo WHERE id = '%s'" % identifier
+query = "UPDATE foo SET value = 'b' WHERE id = '%s'" % identifier
+query = """WITH cte AS (SELECT x FROM foo)
+SELECT x FROM cte WHERE x = '%s'""" % identifier
+# bad alternate forms
+query = "SELECT * FROM foo WHERE id = '" + identifier + "'"
+query = "SELECT * FROM foo WHERE id = '{}'".format(identifier)
+query = f"SELECT * FROM foo WHERE id = {tmp}"
+
+# bad
+cur.execute("SELECT * FROM foo WHERE id = '%s'" % identifier)
+cur.execute("INSERT INTO foo VALUES ('a', 'b', '%s')" % value)
+cur.execute("DELETE FROM foo WHERE id = '%s'" % identifier)
+cur.execute("UPDATE foo SET value = 'b' WHERE id = '%s'" % identifier)
+# bad alternate forms
+cur.execute("SELECT * FROM foo WHERE id = '" + identifier + "'")
+cur.execute("SELECT * FROM foo WHERE id = '{}'".format(identifier))
+cur.execute(f"SELECT * FROM foo WHERE id {tmp}")
+
+# good
+cur.execute("SELECT * FROM foo WHERE id = '%s'", identifier)
+cur.execute("INSERT INTO foo VALUES ('a', 'b', '%s')", value)
+cur.execute("DELETE FROM foo WHERE id = '%s'", identifier)
+cur.execute("UPDATE foo SET value = 'b' WHERE id = '%s'", identifier)
+
+# bug: https://bugs.launchpad.net/bandit/+bug/1479625
+def a():
+    def b():
+        pass
+    return b
+
+a()("SELECT %s FROM foo" % val)
+
+# real world false positives
+choices=[('server_list', _("Select from active instances"))]
+print("delete from the cache as the first argument")

examples/yaml_load.py

@@ -6,7 +6,11 @@ def test_yaml_load():
     ystr = yaml.dump({'a': 1, 'b': 2, 'c': 3})
     y = yaml.load(ystr)
     yaml.dump(y)
-    y = yaml.load(ystr, Loader=yaml.SafeLoader)
+    try:
+        y = yaml.load(ystr, Loader=yaml.CSafeLoader)
+    except AttributeError:
+        # CSafeLoader only exists if you build yaml with LibYAML
+        y = yaml.load(ystr, Loader=yaml.SafeLoader)
 
 
 def test_json_load():

lower-constraints.txt

@@ -24,7 +24,7 @@ pyflakes==0.8.1
 pylint==1.4.5
 python-mimeparse==1.6.0
 python-subunit==1.0.0
-PyYAML==3.12
+PyYAML==3.13
 requests==2.14.2
 requestsexceptions==1.2.0
 six==1.10.0

requirements.txt

@@ -2,6 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 GitPython>=1.0.1 # BSD License (3 clause)
-PyYAML>=3.12 # MIT
+PyYAML>=3.13 # MIT
 six>=1.10.0 # MIT
 stevedore>=1.20.0 # Apache-2.0
+colorama>=0.3.9;platform_system=="Windows" # BSD License (3 clause)

tests/functional/test_functional.py

@@ -15,6 +15,7 @@
 # under the License.
 
 import os
+import sys
 
 import six
 import testtools
@@ -404,11 +405,25 @@ def test_ignore_skip(self):
 
     def test_sql_statements(self):
         '''Test for SQL injection through string building.'''
-        expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 14, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 8, 'MEDIUM': 6, 'HIGH': 0}
-        }
-        self.check_example('sql_statements.py', expect)
+        filename = 'sql_statements{}.py'
+        if sys.version_info <= (3, 6):
+            filename = filename.format('')
+            expect = {
+                'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 14,
+                             'HIGH': 0},
+                'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 8, 'MEDIUM': 6,
+                               'HIGH': 0}
+            }
+        else:
+            filename = filename.format('-py36')
+            expect = {
+                'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 16,
+                             'HIGH': 0},
+                'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 9, 'MEDIUM': 7,
+                               'HIGH': 0}
+            }
+
+        self.check_example(filename, expect)
 
     def test_ssl_insecure_version(self):
         '''Test for insecure SSL protocol versions.'''
@@ -597,8 +612,8 @@ def test_asserts(self):
     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)