Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions bandit/plugins/injection_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@

- cursor.execute("SELECT %s FROM derp;" % var)

Use of str.replace in the string construction can also be dangerous.
For example:

- "SELECT * FROM foo WHERE id = '[VALUE]'".replace("[VALUE]", identifier)

However, such cases are always reported with LOW confidence to compensate
for false positives, since valid uses of str.replace can be common.

:Example:

Expand All @@ -52,6 +59,9 @@
.. versionchanged:: 1.7.3
CWE information added

.. versionchanged:: 1.7.7
Flag when str.replace is used in the string construction

""" # noqa: E501
import ast
import re
Expand All @@ -77,18 +87,20 @@ def _check_string(data):
def _evaluate_ast(node):
wrapper = None
statement = ""
str_replace = False

if isinstance(node._bandit_parent, ast.BinOp):
out = utils.concat_string(node, node._bandit_parent)
wrapper = out[0]._bandit_parent
statement = out[1]
elif (
isinstance(node._bandit_parent, ast.Attribute)
and node._bandit_parent.attr == "format"
):
elif isinstance(
node._bandit_parent, ast.Attribute
) and node._bandit_parent.attr in ("format", "replace"):
statement = node.s
# Hierarchy for "".format() is Wrapper -> Call -> Attribute -> Str
wrapper = node._bandit_parent._bandit_parent._bandit_parent
if node._bandit_parent.attr == "replace":
str_replace = True
elif hasattr(ast, "JoinedStr") and isinstance(
node._bandit_parent, ast.JoinedStr
):
Expand All @@ -108,19 +120,21 @@ def _evaluate_ast(node):
if isinstance(wrapper, ast.Call): # wrapped in "execute" call?
names = ["execute", "executemany"]
name = utils.get_called_name(wrapper)
return (name in names, statement)
return (name in names, statement, str_replace)
else:
return (False, statement)
return (False, statement, str_replace)


@test.checks("Str")
@test.test_id("B608")
def hardcoded_sql_expressions(context):
val = _evaluate_ast(context.node)
if _check_string(val[1]):
execute_call, statement, str_replace = _evaluate_ast(context.node)
if _check_string(statement):
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.MEDIUM if val[0] else bandit.LOW,
confidence=bandit.MEDIUM
if execute_call and not str_replace
else bandit.LOW,
cwe=issue.Cwe.SQL_INJECTION,
text="Possible SQL injection vector through string-based "
"query construction.",
Expand Down
2 changes: 2 additions & 0 deletions examples/sql_statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# bad alternate forms
query = "SELECT * FROM foo WHERE id = '" + identifier + "'"
query = "SELECT * FROM foo WHERE id = '{}'".format(identifier)
query = "SELECT * FROM foo WHERE id = '[VALUE]'".replace("[VALUE]", identifier)

# bad
cur.execute("SELECT * FROM foo WHERE id = '%s'" % identifier)
Expand All @@ -19,6 +20,7 @@
# bad alternate forms
cur.execute("SELECT * FROM foo WHERE id = '" + identifier + "'")
cur.execute("SELECT * FROM foo WHERE id = '{}'".format(identifier))
cur.execute("SELECT * FROM foo WHERE id = '[VALUE]'".replace("[VALUE]", identifier))

# bad f-strings
cur.execute(f"SELECT {column_name} FROM foo WHERE id = 1")
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,12 @@ def test_sql_statements(self):
"SEVERITY": {
"UNDEFINED": 0,
"LOW": 0,
"MEDIUM": 18,
"MEDIUM": 20,
"HIGH": 0,
},
"CONFIDENCE": {
"UNDEFINED": 0,
"LOW": 8,
"LOW": 10,
"MEDIUM": 10,
"HIGH": 0,
},
Expand Down