Skip to content

Commit 8302015

Browse files
MaffoochMaffooch
authored
Importer Close Old Findings: Accommodate different dedupe algorithms (#11729)
* Importer Close Old Findings: Accommodate different dedupe algorithms * Rename close_old_findings_report_Line31.json to close_old_findings_report_line31.json
1 parent b483752 commit 8302015

7 files changed

+676
-25
lines changed

‎dojo/importers/base_importer.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,13 @@ def determine_process_method(
263263
**kwargs,
264264
)
265265

266+
def determine_deduplication_algorithm(self) -> str:
267+
"""
268+
Determines what dedupe algorithm to use for the Test being processed.
269+
:return: A string representing the dedupe algorithm to use.
270+
"""
271+
return self.test.deduplication_algorithm
272+
266273
def update_test_meta(self):
267274
"""
268275
Update the test with some values stored in the kwargs dict. The common

‎dojo/importers/default_importer.py

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -254,29 +254,44 @@ def close_old_findings(
254254
# First check if close old findings is desired
255255
if not self.close_old_findings_toggle:
256256
return []
257-
logger.debug("REIMPORT_SCAN: Closing findings no longer present in scan report")
258-
# Close old active findings that are not reported by this scan.
259-
# Refactoring this to only call test.finding_set.values() once.
260-
findings = findings.values()
261-
mitigated_hash_codes = []
257+
258+
logger.debug("IMPORT_SCAN: Closing findings no longer present in scan report")
259+
# Remove all the findings that are coming from the report already mitigated
262260
new_hash_codes = []
263-
for finding in findings:
264-
new_hash_codes.append(finding["hash_code"])
265-
if finding.get("is_mitigated", None):
266-
mitigated_hash_codes.append(finding["hash_code"])
267-
for hash_code in new_hash_codes:
268-
if hash_code == finding["hash_code"]:
269-
new_hash_codes.remove(hash_code)
261+
new_unique_ids_from_tool = []
262+
for finding in findings.values():
263+
# Do not process closed findings in the report
264+
if finding.get("is_mitigated", False):
265+
continue
266+
# Grab the hash code
267+
if (hash_code := finding.get("hash_code")) is not None:
268+
new_hash_codes.append(hash_code)
269+
if (unique_id_from_tool := finding.get("unique_id_from_tool")) is not None:
270+
new_unique_ids_from_tool.append(unique_id_from_tool)
270271
# Get the initial filtered list of old findings to be closed without
271272
# considering the scope of the product or engagement
272-
old_findings = Finding.objects.exclude(
273-
test=self.test,
274-
).exclude(
275-
hash_code__in=new_hash_codes,
276-
).filter(
273+
old_findings = Finding.objects.filter(
277274
test__test_type=self.test.test_type,
278275
active=True,
279-
)
276+
).exclude(test=self.test)
277+
# Filter further based on the deduplication algorithm set on the test
278+
self.deduplication_algorithm = self.determine_deduplication_algorithm()
279+
if self.deduplication_algorithm in ["hash_code", "legacy"]:
280+
old_findings = old_findings.exclude(
281+
hash_code__in=new_hash_codes,
282+
)
283+
if self.deduplication_algorithm == "unique_id_from_tool":
284+
old_findings = old_findings.exclude(
285+
unique_id_from_tool__in=new_unique_ids_from_tool,
286+
)
287+
if self.deduplication_algorithm == "unique_id_from_tool_or_hash_code":
288+
old_findings = old_findings.exclude(
289+
(Q(hash_code__isnull=False) & Q(hash_code__in=new_hash_codes))
290+
| (
291+
Q(unique_id_from_tool__isnull=False)
292+
& Q(unique_id_from_tool__in=new_unique_ids_from_tool)
293+
),
294+
)
280295
# Accommodate for product scope or engagement scope
281296
if self.close_old_findings_product_scope:
282297
old_findings = old_findings.filter(test__engagement__product=self.test.engagement.product)

‎dojo/importers/default_reimporter.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,6 @@ def process_scan(
147147
test_import_history,
148148
)
149149

150-
def determine_deduplication_algorithm(self) -> str:
151-
"""
152-
Determines what dedupe algorithm to use for the Test being processed.
153-
:return: A string representing the dedupe algorithm to use.
154-
"""
155-
return self.test.deduplication_algorithm
156-
157150
def process_findings(
158151
self,
159152
parsed_findings: list[Finding],

‎unittests/scans/semgrep/close_old_findings_report_line31.json

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
{
2+
"version": "1.97.0",
3+
"results": [
4+
{
5+
"check_id": "python.lang.security.audit.dangerous-subprocess-use-tainted-env-args.dangerous-subprocess-use-tainted-env-args",
6+
"path": "sample/brute.py",
7+
"start": {
8+
"line": 31,
9+
"col": 29,
10+
"offset": 285
11+
},
12+
"end": {
13+
"line": 31,
14+
"col": 58,
15+
"offset": 314
16+
},
17+
"extra": {
18+
"metavars": {
19+
"$FUNC": {
20+
"start": {
21+
"line": 31,
22+
"col": 25,
23+
"offset": 281
24+
},
25+
"end": {
26+
"line": 31,
27+
"col": 28,
28+
"offset": 284
29+
},
30+
"abstract_content": "run"
31+
},
32+
"$CMD": {
33+
"start": {
34+
"line": 31,
35+
"col": 29,
36+
"offset": 285
37+
},
38+
"end": {
39+
"line": 31,
40+
"col": 58,
41+
"offset": 314
42+
},
43+
"abstract_content": "[program username password]"
44+
}
45+
},
46+
"message": "Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.",
47+
"metadata": {
48+
"owasp": [
49+
"A01:2017 - Injection",
50+
"A03:2021 - Injection"
51+
],
52+
"cwe": [
53+
"CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')"
54+
],
55+
"asvs": {
56+
"control_id": "5.3.8 OS Command Injection",
57+
"control_url": "https://github.com/OWASP/ASVS/blob/master/4.0/en/0x13-V5-Validation-Sanitization-Encoding.md#v53-output-encoding-and-injection-prevention-requirements",
58+
"section": "V5: Validation, Sanitization and Encoding Verification Requirements",
59+
"version": "4"
60+
},
61+
"references": [
62+
"https://stackoverflow.com/questions/3172470/actual-meaning-of-shell-true-in-subprocess",
63+
"https://docs.python.org/3/library/subprocess.html",
64+
"https://docs.python.org/3/library/shlex.html",
65+
"https://semgrep.dev/docs/cheat-sheets/python-command-injection/"
66+
],
67+
"category": "security",
68+
"technology": [
69+
"python"
70+
],
71+
"confidence": "MEDIUM",
72+
"cwe2022-top25": true,
73+
"cwe2021-top25": true,
74+
"subcategory": [
75+
"vuln"
76+
],
77+
"likelihood": "MEDIUM",
78+
"impact": "MEDIUM",
79+
"license": "Commons Clause License Condition v1.0[LGPL-2.1-only]",
80+
"vulnerability_class": [
81+
"Command Injection"
82+
],
83+
"source": "https://semgrep.dev/r/python.lang.security.audit.dangerous-subprocess-use-tainted-env-args.dangerous-subprocess-use-tainted-env-args",
84+
"shortlink": "https://sg.run/pLGg",
85+
"semgrep.dev": {
86+
"rule": {
87+
"origin": "community",
88+
"r_id": 27262,
89+
"rule_id": "AbUgrZ",
90+
"rv_id": 928299,
91+
"url": "https://semgrep.dev/playground/r/0bTpAQn/python.lang.security.audit.dangerous-subprocess-use-tainted-env-args.dangerous-subprocess-use-tainted-env-args",
92+
"version_id": "0bTpAQn"
93+
}
94+
}
95+
},
96+
"severity": "ERROR",
97+
"fingerprint": "1d0cfd65fc17a97773e848ec31ddd8a580c5fb54914f03bebe4f934a16d5d45f22cfd3ab1b9f3ab9871da06569701e38ea2b518d2a1aa397cd5d7ecbd699d4a4_0",
98+
"lines": " result = subprocess.run([program, username, password], stdout=subprocess.DEVNULL)",
99+
"is_ignored": false,
100+
"dataflow_trace": {
101+
"taint_source": [
102+
"CliLoc",
103+
[
104+
{
105+
"path": "sample/brute.py",
106+
"start": {
107+
"line": 6,
108+
"col": 11,
109+
"offset": 64
110+
},
111+
"end": {
112+
"line": 6,
113+
"col": 19,
114+
"offset": 72
115+
}
116+
},
117+
"sys.argv"
118+
]
119+
],
120+
"intermediate_vars": [
121+
{
122+
"location": {
123+
"path": "sample/brute.py",
124+
"start": {
125+
"line": 6,
126+
"col": 1,
127+
"offset": 54
128+
},
129+
"end": {
130+
"line": 6,
131+
"col": 8,
132+
"offset": 61
133+
}
134+
},
135+
"content": "program"
136+
},
137+
{
138+
"location": {
139+
"path": "sample/brute.py",
140+
"start": {
141+
"line": 6,
142+
"col": 1,
143+
"offset": 54
144+
},
145+
"end": {
146+
"line": 6,
147+
"col": 8,
148+
"offset": 61
149+
}
150+
},
151+
"content": "program"
152+
}
153+
],
154+
"taint_sink": [
155+
"CliLoc",
156+
[
157+
{
158+
"path": "sample/brute.py",
159+
"start": {
160+
"line": 31,
161+
"col": 29,
162+
"offset": 285
163+
},
164+
"end": {
165+
"line": 31,
166+
"col": 58,
167+
"offset": 314
168+
}
169+
},
170+
"[program, username, password]"
171+
]
172+
]
173+
},
174+
"engine_kind": "OSS",
175+
"validation_state": "NO_VALIDATOR"
176+
}
177+
}
178+
],
179+
"errors": [],
180+
"paths": {
181+
"scanned": [
182+
"README.md",
183+
"sample/brute.py",
184+
"sample/findmysecrets.go",
185+
"sample/function.go",
186+
"sample/go.mod",
187+
"sample/session.go",
188+
"sample/sqli.go"
189+
]
190+
},
191+
"interfile_languages_used": [],
192+
"skipped_rules": []
193+
}

0 commit comments

Comments
 (0)