Skip to content

Commit dec36c3

Browse files
andrecsilvaandrecsilva
authored
Fixed a bug, adjusted tests (#615)
* Fixed a bug, adjusted tests * Added extra test * Small refactor
1 parent 945e627 commit dec36c3

File tree

4 files changed

+455
-9
lines changed

4 files changed

+455
-9
lines changed

‎src/core_codemods/sql_parameterization.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
177177
for call, linearized_query in find_queries.calls.items():
178178
# filter node
179179
if not self.node_is_selected(call):
180-
return tree
180+
continue
181181

182182
# Step (2)
183183
ep = ExtractParameters(self.context, linearized_query)

‎tests/codemods/sonar/test_sonar_sql_parameterization.py‎

Lines changed: 74 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
import json
22
from pathlib import Path
33

4-
import pytest
5-
64
from codemodder.codemods.test import BaseSASTCodemodTest
75
from core_codemods.sonar.sonar_sql_parameterization import SonarSQLParameterization
86

9-
SAMPLE_FILE_PATH = (
10-
Path(__file__).parents[2] / "samples" / "sonar" / "sql_parameterization.json"
11-
)
7+
SAMPLE_FILE_PATH = [
8+
(Path(__file__).parents[2] / "samples" / "sonar" / "sql_parameterization.json"),
9+
(Path(__file__).parents[2] / "samples" / "sonar" / "sql_parameterization2.json"),
10+
]
1211

1312

1413
class TestSonarSQLParameterization(BaseSASTCodemodTest):
@@ -66,8 +65,76 @@ def f():
6665
}
6766
self.run_and_assert(tmpdir, input_code, expected, results=json.dumps(issues))
6867

69-
@pytest.mark.xfail(reason="TODO: See issue #607")
7068
def test_more_complicated_example(self, tmpdir):
69+
input_code = """
70+
from django.core.signals import request_finished
71+
from django.dispatch import receiver
72+
from django.views.decorators.csrf import csrf_exempt
73+
from django.shortcuts import redirect, render
74+
75+
import sqlite3
76+
77+
connection = sqlite3.connect(":memory:")
78+
connection.cursor().execute("CREATE TABLE Users (name, phone)")
79+
connection.cursor().execute("INSERT INTO Users VALUES ('Jenny','867-5309')")
80+
81+
@csrf_exempt
82+
@receiver(request_finished)
83+
def bad_query(request):
84+
if request.method == 'POST':
85+
name = request.POST.get('name')
86+
phone = request.POST.get('phone')
87+
88+
query = "SELECT * FROM Users WHERE name ='" + name + "' AND phone = '" + phone + "'"
89+
result = connection.cursor().execute(query)
90+
return render(request, 'result.html', {'result': result})
91+
else:
92+
return redirect('/')
93+
""".lstrip(
94+
"\n"
95+
)
96+
expected = """
97+
from django.core.signals import request_finished
98+
from django.dispatch import receiver
99+
from django.views.decorators.csrf import csrf_exempt
100+
from django.shortcuts import redirect, render
101+
102+
import sqlite3
103+
104+
connection = sqlite3.connect(":memory:")
105+
connection.cursor().execute("CREATE TABLE Users (name, phone)")
106+
connection.cursor().execute("INSERT INTO Users VALUES ('Jenny','867-5309')")
107+
108+
@csrf_exempt
109+
@receiver(request_finished)
110+
def bad_query(request):
111+
if request.method == 'POST':
112+
name = request.POST.get('name')
113+
phone = request.POST.get('phone')
114+
115+
query = "SELECT * FROM Users WHERE name =?" + " AND phone = ?"
116+
result = connection.cursor().execute(query, (name, phone, ))
117+
return render(request, 'result.html', {'result': result})
118+
else:
119+
return redirect('/')
120+
""".lstrip(
121+
"\n"
122+
)
123+
124+
issues = json.loads(SAMPLE_FILE_PATH[0].read_text())
125+
126+
filename = Path(tmpdir) / "introduction" / "new_view.py"
127+
filename.parent.mkdir(parents=True)
128+
129+
self.run_and_assert(
130+
tmpdir,
131+
input_code,
132+
expected,
133+
files=[filename],
134+
results=json.dumps(issues),
135+
)
136+
137+
def test_regression(self, tmpdir):
71138
input_code = """
72139
from django.shortcuts import redirect
73140
from django.http import HttpResponse
@@ -115,7 +182,7 @@ def do_useful_things(request):
115182
"\n"
116183
)
117184

118-
issues = json.loads(SAMPLE_FILE_PATH.read_text())
185+
issues = json.loads(SAMPLE_FILE_PATH[1].read_text())
119186

120187
filename = Path(tmpdir) / "introduction" / "new_view.py"
121188
filename.parent.mkdir(parents=True)

‎tests/samples/sonar/sql_parameterization.json‎

Lines changed: 244 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,244 @@
1-
{"total":2,"p":1,"ps":100,"paging":{"pageIndex":1,"pageSize":100,"total":2},"effortTotal":35,"debtTotal":35,"issues":[{"key":"AY_KW3q9kpLwWuMztk6B","rule":"python:S6552","severity":"MAJOR","component":"drdavella_pygoat-sonar2:introduction/new_view.py","project":"drdavella_pygoat-sonar2","line":13,"hash":"91d1a8baa6977afdf844ab3f2870df56","textRange":{"startLine":13,"endLine":13,"startOffset":0,"endOffset":27},"flows":[],"status":"OPEN","message":"Move this \u0027@receiver\u0027 decorator to the top of the other decorators.","effort":"5min","debt":"5min","tags":[],"creationDate":"2024-05-30T18:34:05+0200","updateDate":"2024-05-30T18:35:24+0200","type":"BUG","organization":"drdavella","pullRequest":"5","cleanCodeAttribute":"LOGICAL","cleanCodeAttributeCategory":"INTENTIONAL","impacts":[{"softwareQuality":"RELIABILITY","severity":"MEDIUM"}]},{"key":"AY_KW3q9kpLwWuMztk6D","rule":"pythonsecurity:S3649","severity":"BLOCKER","component":"drdavella_pygoat-sonar2:introduction/new_view.py","project":"drdavella_pygoat-sonar2","line":20,"hash":"cc503242d422d9ba9c4f02e0c7e243cb","textRange":{"startLine":20,"endLine":20,"startOffset":17,"endOffset":51},"flows":[{"locations":[{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":20,"endLine":20,"startOffset":17,"endOffset":51},"msg":"Sink: this invocation is not safe; a malicious value can be used as argument"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":8,"endOffset":92},"msg":"A malicious value can be assigned to variable ‘query’"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":16,"endOffset":92},"msg":"This concatenation can propagate malicious content to the newly created string"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":54,"endOffset":58},"msg":"The malicious content is concatenated into the string"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":16,"endLine":16,"startOffset":8,"endOffset":39},"msg":"A malicious value can be assigned to variable ‘name’"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":16,"endLine":16,"startOffset":15,"endOffset":39},"msg":"Source: a user can craft an HTTP request with malicious content"}]},{"locations":[{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":20,"endLine":20,"startOffset":17,"endOffset":51},"msg":"Sink: this invocation is not safe; a malicious value can be used as argument"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":8,"endOffset":92},"msg":"A malicious value can be assigned to variable ‘query’"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":16,"endOffset":92},"msg":"This concatenation can propagate malicious content to the newly created string"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":19,"endLine":19,"startOffset":81,"endOffset":86},"msg":"The malicious content is concatenated into the string"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":17,"endLine":17,"startOffset":8,"endOffset":41},"msg":"A malicious value can be assigned to variable ‘phone’"},{"component":"drdavella_pygoat-sonar2:introduction/new_view.py","textRange":{"startLine":17,"endLine":17,"startOffset":16,"endOffset":41},"msg":"Source: a user can craft an HTTP request with malicious content"}]}],"status":"OPEN","message":"Change this code to not construct SQL queries directly from user-controlled data.","effort":"30min","debt":"30min","tags":["cwe","sql"],"creationDate":"2024-05-30T18:34:05+0200","updateDate":"2024-05-30T18:35:24+0200","type":"VULNERABILITY","organization":"drdavella","pullRequest":"5","cleanCodeAttribute":"COMPLETE","cleanCodeAttributeCategory":"INTENTIONAL","impacts":[{"softwareQuality":"SECURITY","severity":"HIGH"}]}],"components":[{"organization":"drdavella","key":"drdavella_pygoat-sonar2:introduction/new_view.py","uuid":"AY_KW3gxkpLwWuMztk5a","enabled":true,"qualifier":"FIL","name":"new_view.py","longName":"introduction/new_view.py","path":"introduction/new_view.py","pullRequest":"5"},{"organization":"drdavella","key":"drdavella_pygoat-sonar2","uuid":"AY_KWyTh-nOrCSyUsFgG","enabled":true,"qualifier":"TRK","name":"pygoat-sonar","longName":"pygoat-sonar","pullRequest":"5"}],"organizations":[{"key":"drdavella","name":"Dan D\u0027Avella"}],"facets":[]}
1+
{
2+
"total": 2,
3+
"p": 1,
4+
"ps": 100,
5+
"paging": {
6+
"pageIndex": 1,
7+
"pageSize": 100,
8+
"total": 2
9+
},
10+
"effortTotal": 35,
11+
"debtTotal": 35,
12+
"issues": [
13+
{
14+
"key": "AY_KW3q9kpLwWuMztk6B",
15+
"rule": "python:S6552",
16+
"severity": "MAJOR",
17+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
18+
"project": "drdavella_pygoat-sonar2",
19+
"line": 13,
20+
"hash": "91d1a8baa6977afdf844ab3f2870df56",
21+
"textRange": {
22+
"startLine": 13,
23+
"endLine": 13,
24+
"startOffset": 0,
25+
"endOffset": 27
26+
},
27+
"flows": [],
28+
"status": "OPEN",
29+
"message": "Move this '@receiver' decorator to the top of the other decorators.",
30+
"effort": "5min",
31+
"debt": "5min",
32+
"tags": [],
33+
"creationDate": "2024-05-30T18:34:05+0200",
34+
"updateDate": "2024-05-30T18:35:24+0200",
35+
"type": "BUG",
36+
"organization": "drdavella",
37+
"pullRequest": "5",
38+
"cleanCodeAttribute": "LOGICAL",
39+
"cleanCodeAttributeCategory": "INTENTIONAL",
40+
"impacts": [
41+
{
42+
"softwareQuality": "RELIABILITY",
43+
"severity": "MEDIUM"
44+
}
45+
]
46+
},
47+
{
48+
"key": "AY_KW3q9kpLwWuMztk6D",
49+
"rule": "pythonsecurity:S3649",
50+
"severity": "BLOCKER",
51+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
52+
"project": "drdavella_pygoat-sonar2",
53+
"line": 20,
54+
"hash": "cc503242d422d9ba9c4f02e0c7e243cb",
55+
"textRange": {
56+
"startLine": 20,
57+
"endLine": 20,
58+
"startOffset": 17,
59+
"endOffset": 51
60+
},
61+
"flows": [
62+
{
63+
"locations": [
64+
{
65+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
66+
"textRange": {
67+
"startLine": 20,
68+
"endLine": 20,
69+
"startOffset": 17,
70+
"endOffset": 51
71+
},
72+
"msg": "Sink: this invocation is not safe; a malicious value can be used as argument"
73+
},
74+
{
75+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
76+
"textRange": {
77+
"startLine": 19,
78+
"endLine": 19,
79+
"startOffset": 8,
80+
"endOffset": 92
81+
},
82+
"msg": "A malicious value can be assigned to variable ‘query’"
83+
},
84+
{
85+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
86+
"textRange": {
87+
"startLine": 19,
88+
"endLine": 19,
89+
"startOffset": 16,
90+
"endOffset": 92
91+
},
92+
"msg": "This concatenation can propagate malicious content to the newly created string"
93+
},
94+
{
95+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
96+
"textRange": {
97+
"startLine": 19,
98+
"endLine": 19,
99+
"startOffset": 54,
100+
"endOffset": 58
101+
},
102+
"msg": "The malicious content is concatenated into the string"
103+
},
104+
{
105+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
106+
"textRange": {
107+
"startLine": 16,
108+
"endLine": 16,
109+
"startOffset": 8,
110+
"endOffset": 39
111+
},
112+
"msg": "A malicious value can be assigned to variable ‘name’"
113+
},
114+
{
115+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
116+
"textRange": {
117+
"startLine": 16,
118+
"endLine": 16,
119+
"startOffset": 15,
120+
"endOffset": 39
121+
},
122+
"msg": "Source: a user can craft an HTTP request with malicious content"
123+
}
124+
]
125+
},
126+
{
127+
"locations": [
128+
{
129+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
130+
"textRange": {
131+
"startLine": 20,
132+
"endLine": 20,
133+
"startOffset": 17,
134+
"endOffset": 51
135+
},
136+
"msg": "Sink: this invocation is not safe; a malicious value can be used as argument"
137+
},
138+
{
139+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
140+
"textRange": {
141+
"startLine": 19,
142+
"endLine": 19,
143+
"startOffset": 8,
144+
"endOffset": 92
145+
},
146+
"msg": "A malicious value can be assigned to variable ‘query’"
147+
},
148+
{
149+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
150+
"textRange": {
151+
"startLine": 19,
152+
"endLine": 19,
153+
"startOffset": 16,
154+
"endOffset": 92
155+
},
156+
"msg": "This concatenation can propagate malicious content to the newly created string"
157+
},
158+
{
159+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
160+
"textRange": {
161+
"startLine": 19,
162+
"endLine": 19,
163+
"startOffset": 81,
164+
"endOffset": 86
165+
},
166+
"msg": "The malicious content is concatenated into the string"
167+
},
168+
{
169+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
170+
"textRange": {
171+
"startLine": 17,
172+
"endLine": 17,
173+
"startOffset": 8,
174+
"endOffset": 41
175+
},
176+
"msg": "A malicious value can be assigned to variable ‘phone’"
177+
},
178+
{
179+
"component": "drdavella_pygoat-sonar2:introduction/new_view.py",
180+
"textRange": {
181+
"startLine": 17,
182+
"endLine": 17,
183+
"startOffset": 16,
184+
"endOffset": 41
185+
},
186+
"msg": "Source: a user can craft an HTTP request with malicious content"
187+
}
188+
]
189+
}
190+
],
191+
"status": "OPEN",
192+
"message": "Change this code to not construct SQL queries directly from user-controlled data.",
193+
"effort": "30min",
194+
"debt": "30min",
195+
"tags": [
196+
"cwe",
197+
"sql"
198+
],
199+
"creationDate": "2024-05-30T18:34:05+0200",
200+
"updateDate": "2024-05-30T18:35:24+0200",
201+
"type": "VULNERABILITY",
202+
"organization": "drdavella",
203+
"pullRequest": "5",
204+
"cleanCodeAttribute": "COMPLETE",
205+
"cleanCodeAttributeCategory": "INTENTIONAL",
206+
"impacts": [
207+
{
208+
"softwareQuality": "SECURITY",
209+
"severity": "HIGH"
210+
}
211+
]
212+
}
213+
],
214+
"components": [
215+
{
216+
"organization": "drdavella",
217+
"key": "drdavella_pygoat-sonar2:introduction/new_view.py",
218+
"uuid": "AY_KW3gxkpLwWuMztk5a",
219+
"enabled": true,
220+
"qualifier": "FIL",
221+
"name": "new_view.py",
222+
"longName": "introduction/new_view.py",
223+
"path": "introduction/new_view.py",
224+
"pullRequest": "5"
225+
},
226+
{
227+
"organization": "drdavella",
228+
"key": "drdavella_pygoat-sonar2",
229+
"uuid": "AY_KWyTh-nOrCSyUsFgG",
230+
"enabled": true,
231+
"qualifier": "TRK",
232+
"name": "pygoat-sonar",
233+
"longName": "pygoat-sonar",
234+
"pullRequest": "5"
235+
}
236+
],
237+
"organizations": [
238+
{
239+
"key": "drdavella",
240+
"name": "Dan D'Avella"
241+
}
242+
],
243+
"facets": []
244+
}

0 commit comments

Comments
 (0)