Merge branch 'master' into patch-1

README.rst

@@ -7,6 +7,10 @@
     :target: https://travis-ci.org/PyCQA/bandit/
     :alt: Build Status
 
+.. image:: https://readthedocs.org/projects/bandit/badge/?version=latest
+    :target: https://readthedocs.org/projects/bandit/
+    :alt: Docs Status
+
 .. image:: https://img.shields.io/pypi/v/bandit.svg
     :target: https://pypi.org/project/bandit/
     :alt: Latest Version

bandit/core/manager.py

@@ -20,8 +20,11 @@
 import logging
 import os
 import sys
+import tokenize
 import traceback
 
+import six
+
 from bandit.core import constants as b_constants
 from bandit.core import extension_loader
 from bandit.core import issue
@@ -271,10 +274,18 @@ def _parse_file(self, fname, fdata, new_files_list):
             if self.ignore_nosec:
                 nosec_lines = set()
             else:
-                nosec_lines = set(
-                    lineno + 1 for
-                    (lineno, line) in enumerate(lines)
-                    if b'#nosec' in line or b'# nosec' in line)
+                try:
+                    fdata.seek(0)
+                    if six.PY2:
+                        tokens = tokenize.generate_tokens(fdata.readline)
+                    else:
+                        tokens = tokenize.tokenize(fdata.readline)
+                    nosec_lines = set(
+                        lineno for toktype, tokval, (lineno, _), _, _ in tokens
+                        if toktype == tokenize.COMMENT and
+                        '#nosec' in tokval or '# nosec' in tokval)
+                except tokenize.TokenError as e:
+                    nosec_lines = set()
             score = self._execute_ast_visitor(fname, data, nosec_lines)
             self.scores.append(score)
             self.metrics.count_issues([score, ])

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/nosec.py

@@ -3,3 +3,5 @@
                  shell=True)
 subprocess.Popen('/bin/ls *',
                  shell=True)  #nosec (on the specific kwarg line)
+subprocess.Popen('#nosec', shell=True)
+subprocess.Popen('/bin/ls *', shell=True) # type: … # nosec # noqa: E501 ; pylint: disable=line-too-long

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

pylintrc

@@ -12,6 +12,7 @@
 # TODO(browne): fix these in the future
 # C0103: invalid-name
 # E1101: no-member
+# E1111: assignment-from-no-return
 # R0902: too-many-instance-attributes
 # R0912: too-many-branches
 # R0913: too-many-arguments
@@ -26,7 +27,7 @@
 # W0613: unused-argument
 # W0621: redefined-outer-name
 # W0703: broad-except
-disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0613,W0621,W0703
+disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,E1111,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0613,W0621,W0703
 
 [Basic]
 # Variable names can be 1 to 31 characters long, with lowercase and underscores

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)
 
@@ -708,8 +723,8 @@ def test_flask_debug_true(self):
 
     def test_nosec(self):
         expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0}
+            'SEVERITY': {'UNDEFINED': 0, 'LOW': 2, 'MEDIUM': 0, 'HIGH': 0},
+            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
         }
         self.check_example('nosec.py', expect)
 

tests/functional/test_runtime.py

@@ -15,6 +15,7 @@
 import os
 import subprocess
 
+import six
 import testtools
 
 
@@ -111,11 +112,13 @@ def test_example_nonsense2(self):
             ['bandit', ], ['nonsense2.py', ]
         )
         self.assertEqual(0, retcode)
-        self.assertIn(
-            "Exception occurred when executing tests against", output
-        )
         self.assertIn("Files skipped (1):", output)
-        self.assertIn("nonsense2.py (exception while scanning file)", output)
+        if six.PY2:
+            self.assertIn("nonsense2.py (exception while scanning file)",
+                          output)
+        else:
+            self.assertIn("nonsense2.py (syntax error while parsing AST",
+                          output)
 
     def test_example_imports(self):
         (retcode, output) = self._test_example(['bandit', ], ['imports.py', ])