Skip to content

Commit a2a1eb0

Browse files
andrecsilvaandrecsilva
authored
Adds sonar version of SQLParameterizer codemod (#495)
* Fixed issue with multiple class with injections * Sonar sql parameterization codemod with tests
1 parent d16ff03 commit a2a1eb0

9 files changed

Lines changed: 2311 additions & 2045 deletions

File tree

‎integration_tests/sonar/test_sonar_sql_parameterization.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from codemodder.codemods.test import SonarIntegrationTest
2+
from core_codemods.sonar.sonar_sql_parameterization import SonarSQLParameterization
3+
from core_codemods.sql_parameterization import SQLQueryParameterizationTransformer
4+
5+
6+
class TestSonarSQLParameterization(SonarIntegrationTest):
7+
codemod = SonarSQLParameterization
8+
code_path = "tests/samples/fix_sonar_sql_parameterization.py"
9+
replacement_lines = [
10+
(11, ' sql = """SELECT user FROM users WHERE user = ?"""\n'),
11+
(14, " conn.cursor().execute(sql, ((user), ))\n"),
12+
]
13+
expected_diff = """\
14+
---
15+
+++
16+
@@ -8,7 +8,7 @@
17+
@app.route("/example")
18+
def f():
19+
user = request.args["user"]
20+
- sql = \"\"\"SELECT user FROM users WHERE user = \\'%s\\'\"\"\"
21+
+ sql = \"\"\"SELECT user FROM users WHERE user = ?\"\"\"
22+
23+
conn = sqlite3.connect("example")
24+
- conn.cursor().execute(sql % (user))
25+
+ conn.cursor().execute(sql, ((user), ))
26+
"""
27+
expected_line_change = "14"
28+
change_description = SQLQueryParameterizationTransformer.change_description

‎src/codemodder/scripts/generate_docs.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ class DocMetadata:
294294
"url-sandbox-S5144",
295295
"fix-float-equality-S1244",
296296
"fix-math-isclose-S6727",
297+
"sql-parameterization-S3649",
297298
]
298299
SONAR_CODEMODS = {
299300
name: DocMetadata(

‎src/core_codemods/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
SonarRemoveAssertionInPytestRaises,
6868
)
6969
from .sonar.sonar_secure_random import SonarSecureRandom
70+
from .sonar.sonar_sql_parameterization import SonarSQLParameterization
7071
from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp
7172
from .sonar.sonar_url_sandbox import SonarUrlSandbox
7273
from .sql_parameterization import SQLQueryParameterization
@@ -166,6 +167,7 @@
166167
SonarUrlSandbox,
167168
SonarFixFloatEquality,
168169
SonarFixMathIsClose,
170+
SonarSQLParameterization,
169171
],
170172
)
171173

‎src/core_codemods/sonar/sonar_sql_parameterization.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
from core_codemods.sonar.api import SonarCodemod
2+
from core_codemods.sql_parameterization import SQLQueryParameterization
3+
4+
SonarSQLParameterization = SonarCodemod.from_core_codemod(
5+
name="sql-parameterization-S3649",
6+
other=SQLQueryParameterization,
7+
rule_id="pythonsecurity:S3649",
8+
rule_name="Database queries should not be vulnerable to injection attacks",
9+
rule_url="https://rules.sonarsource.com/python/RSPEC-3649/",
10+
)

‎src/core_codemods/sql_parameterization.py‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def __init__(
126126
LibcstResultTransformer.__init__(self, *codemod_args, **codemod_kwargs)
127127
UtilsMixin.__init__(
128128
self,
129-
[],
129+
codemod_args[1],
130130
line_exclude=self.file_context.line_exclude,
131131
line_include=self.file_context.line_include,
132132
)
@@ -165,25 +165,25 @@ def _build_param_element(self, prepend, middle, append, linearized_query):
165165

166166
def transform_module_impl(self, tree: cst.Module) -> cst.Module:
167167
"""
168-
The transformation is composed of 3 steps, each step is done by a codemod/visitor: (1) FindQueryCalls, (2) ExtractParameters, and (3) SQLQueryParameterization.
168+
The transformation is composed of 3 steps, each step is done by a codemod/visitor: (1) FindQueryCalls, (2) ExtractParameters, and (3) _fix_injection
169169
Step (1) finds the `execute` calls and linearizing the query argument. Step (2) extracts the expressions that are parameters to the query.
170170
Step (3) swaps the parameters in the query for `?` tokens and passes them as an arguments for the `execute` call. At the end of the transformation, the `CleanCode` codemod is executed to remove leftover empty strings and unused variables.
171171
"""
172+
173+
# Step (1)
172174
find_queries = FindQueryCalls(self.context)
173175
tree.visit(find_queries)
174176

175-
result = tree
176177
for call, linearized_query in find_queries.calls.items():
177-
# filter by line includes/excludes
178-
call_pos = self.node_position(call)
179-
if not self.filter_by_path_includes_or_excludes(call_pos):
180-
break
178+
# filter node
179+
if not self.node_is_selected(call):
180+
return tree
181181

182-
# Step (3)
182+
# Step (2)
183183
ep = ExtractParameters(self.context, linearized_query)
184184
tree.visit(ep)
185185

186-
# Step (4) - build tuple elements and fix injection
186+
# Step (3)
187187
params_elements: list[cst.Element] = []
188188
for start, middle, end in ep.injection_patterns:
189189
prepend, append = self._fix_injection(
@@ -246,7 +246,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
246246
value=new_raw_value
247247
)
248248

249-
result = result.visit(ReplaceNodes(new_changed_nodes))
249+
result = tree.visit(ReplaceNodes(new_changed_nodes))
250250
self.changed_nodes = {}
251251
line_number = self.get_metadata(PositionProvider, call).start.line
252252
self.file_context.codemod_changes.append(
@@ -258,7 +258,12 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
258258
# Normalization and cleanup
259259
result = CleanCode(self.context).transform_module(result)
260260

261-
return result
261+
# return after a single change
262+
return result
263+
return tree
264+
265+
def should_allow_multiple_passes(self) -> bool:
266+
return True
262267

263268
def _fix_injection(
264269
self,

‎tests/codemods/sonar/test_sonar_sql_parameterization.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import json
2+
3+
from codemodder.codemods.test import BaseSASTCodemodTest
4+
from core_codemods.sonar.sonar_sql_parameterization import SonarSQLParameterization
5+
6+
7+
class TestSonarSQLParameterization(BaseSASTCodemodTest):
8+
codemod = SonarSQLParameterization
9+
tool = "sonar"
10+
11+
def test_name(self):
12+
assert self.codemod.name == "sql-parameterization-S3649"
13+
14+
def test_simple(self, tmpdir):
15+
input_code = """
16+
import sqlite3
17+
18+
from flask import Flask, request
19+
20+
app = Flask(__name__)
21+
22+
@app.route("/example")
23+
def f():
24+
user = request.args["user"]
25+
sql = '''SELECT user FROM users WHERE user = \'%s\' '''
26+
27+
conn = sqlite3.connect("example")
28+
conn.cursor().execute(sql % (user))
29+
"""
30+
expected = """
31+
import sqlite3
32+
33+
from flask import Flask, request
34+
35+
app = Flask(__name__)
36+
37+
@app.route("/example")
38+
def f():
39+
user = request.args["user"]
40+
sql = '''SELECT user FROM users WHERE user = ? '''
41+
42+
conn = sqlite3.connect("example")
43+
conn.cursor().execute(sql, ((user), ))
44+
"""
45+
issues = {
46+
"issues": [
47+
{
48+
"rule": "pythonsecurity:S3649",
49+
"status": "OPEN",
50+
"component": "code.py",
51+
"textRange": {
52+
"startLine": 14,
53+
"endLine": 14,
54+
"startOffset": 4,
55+
"endOffset": 39,
56+
},
57+
}
58+
]
59+
}
60+
self.run_and_assert(tmpdir, input_code, expected, results=json.dumps(issues))

‎tests/codemods/test_sql_parameterization.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,39 @@ def test_multiple(self, tmpdir):
4848
"""
4949
self.run_and_assert(tmpdir, input_code, expected)
5050

51+
def test_multiple_injectable_calls(self, tmpdir):
52+
input_code = """
53+
import sqlite3
54+
55+
def f(user):
56+
sql = '''SELECT user FROM users WHERE user = \'%s\' '''
57+
58+
conn = sqlite3.connect('example')
59+
conn.cursor().execute(sql % (user))
60+
61+
def g(user):
62+
sql = "SELECT user FROM users WHERE user = '" + user + "'"
63+
64+
conn = sqlite3.connect('example')
65+
conn.cursor().execute(sql)
66+
"""
67+
expected = """
68+
import sqlite3
69+
70+
def f(user):
71+
sql = '''SELECT user FROM users WHERE user = ? '''
72+
73+
conn = sqlite3.connect('example')
74+
conn.cursor().execute(sql, ((user), ))
75+
76+
def g(user):
77+
sql = "SELECT user FROM users WHERE user = ?"
78+
79+
conn = sqlite3.connect('example')
80+
conn.cursor().execute(sql, (user, ))
81+
"""
82+
self.run_and_assert(tmpdir, input_code, expected, num_changes=2)
83+
5184
def test_simple_with_quotes_in_middle(self, tmpdir):
5285
input_code = """
5386
import sqlite3

‎tests/samples/fix_sonar_sql_parameterization.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import sqlite3
2+
3+
from flask import Flask, request
4+
5+
app = Flask(__name__)
6+
7+
8+
@app.route("/example")
9+
def f():
10+
user = request.args["user"]
11+
sql = """SELECT user FROM users WHERE user = \'%s\'"""
12+
13+
conn = sqlite3.connect("example")
14+
conn.cursor().execute(sql % (user))

0 commit comments

Comments
 (0)