diff --git a/bandit/plugins/injection_sql.py b/bandit/plugins/injection_sql.py index afb63a5b8..3efa0a5dc 100644 --- a/bandit/plugins/injection_sql.py +++ b/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'] diff --git a/examples/sql_statements-py36.py b/examples/sql_statements-py36.py new file mode 100644 index 000000000..590320528 --- /dev/null +++ b/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") diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py index 0a7af9619..60010a56a 100644 --- a/tests/functional/test_functional.py +++ b/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.'''