diff --git a/bandit/plugins/injection_sql.py b/bandit/plugins/injection_sql.py index c69750ca1..dd9b5c0ed 100644 --- a/bandit/plugins/injection_sql.py +++ b/bandit/plugins/injection_sql.py @@ -92,8 +92,18 @@ def _evaluate_ast(node): elif hasattr(ast, "JoinedStr") and isinstance( node._bandit_parent, ast.JoinedStr ): - statement = node.s - wrapper = node._bandit_parent._bandit_parent + substrings = [ + child + for child in node._bandit_parent.values + if isinstance(child, ast.Str) + ] + # JoinedStr consists of list of Constant and FormattedValue + # instances. Let's perform one test for the whole string + # and abandon all parts except the first one to raise one + # failed test instead of many for the same SQL statement. + if substrings and node == substrings[0]: + statement = "".join([str(child.s) for child in substrings]) + wrapper = node._bandit_parent._bandit_parent if isinstance(wrapper, ast.Call): # wrapped in "execute" call? names = ["execute", "executemany"] diff --git a/examples/sql_multiline_statements.py b/examples/sql_multiline_statements.py new file mode 100644 index 000000000..1f078090e --- /dev/null +++ b/examples/sql_multiline_statements.py @@ -0,0 +1,123 @@ +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 = {identifier} +""" + +# 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)) + +# bad with f-string +query = f""" +SELECT * +FROM foo +WHERE id = {identifier} +""" +query = f""" +SELECT * +FROM foo +WHERE id = {identifier} +""" + +query = f""" +SELECT * +FROM foo +WHERE id = {identifier}""" +query = f""" +SELECT * +FROM foo +WHERE id = {identifier}""" + +cur.execute(f""" +SELECT + {column_name} +FROM foo +WHERE id = 1""") + +cur.execute(f""" +SELECT + {a + b} +FROM foo +WHERE id = 1""") + +cur.execute(f""" +INSERT INTO + {table_name} +VALUES (1)""") +cur.execute(f""" +UPDATE {table_name} +SET id = 1""") + +# implicit concatenation mixed with f-strings +cur.execute("SELECT " + f"{column_name} " + "FROM foo " + "WHERE id = 1" + ) +cur.execute("INSERT INTO " + f"{table_name} " + "VALUES (1)") +cur.execute(f"UPDATE {table_name} " + "SET id = 1") + +# 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) diff --git a/examples/sql_statements.py b/examples/sql_statements.py index 1fabb7090..ff88a926b 100644 --- a/examples/sql_statements.py +++ b/examples/sql_statements.py @@ -20,6 +20,12 @@ cur.execute("SELECT * FROM foo WHERE id = '" + identifier + "'") cur.execute("SELECT * FROM foo WHERE id = '{}'".format(identifier)) +# bad f-strings +cur.execute(f"SELECT {column_name} FROM foo WHERE id = 1") +cur.execute(f"SELECT {a + b} FROM foo WHERE id = 1") +cur.execute(f"INSERT INTO {table_name} VALUES (1)") +cur.execute(f"UPDATE {table_name} SET id = 1") + # good cur.execute("SELECT * FROM foo WHERE id = '%s'", identifier) cur.execute("INSERT INTO foo VALUES ('a', 'b', '%s')", value) diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py index f30ca25f8..e1d10a956 100644 --- a/tests/functional/test_functional.py +++ b/tests/functional/test_functional.py @@ -439,18 +439,40 @@ def test_sql_statements(self): "SEVERITY": { "UNDEFINED": 0, "LOW": 0, - "MEDIUM": 14, + "MEDIUM": 18, "HIGH": 0, }, "CONFIDENCE": { "UNDEFINED": 0, "LOW": 8, - "MEDIUM": 6, + "MEDIUM": 10, "HIGH": 0, }, } self.check_example("sql_statements.py", expect) + def test_multiline_sql_statements(self): + """ + Test for SQL injection through string building using + multi-line strings. + """ + example_file = "sql_multiline_statements.py" + expect = { + "SEVERITY": { + "UNDEFINED": 0, + "LOW": 0, + "MEDIUM": 26, + "HIGH": 0, + }, + "CONFIDENCE": { + "UNDEFINED": 0, + "LOW": 13, + "MEDIUM": 13, + "HIGH": 0, + }, + } + self.check_example(example_file, expect) + def test_ssl_insecure_version(self): """Test for insecure SSL protocol versions.""" expect = {