Skip to content

Commit 2a73bf0

Browse files
fix(test): handle nh3 rel=noopener and escape XSS in create_bleached_link
- Use escape() when building HTML in create_bleached_link so attribute values are properly encoded before nh3 parses them (prevents raw tags in href/title when user-supplied content contains HTML) - Add rel="noopener noreferrer" to all expected link strings in tests (nh3 automatically injects this on target="_blank" links) - Replace exact-output XSS assertion with semantic safety checks
1 parent 3671516 commit 2a73bf0

2 files changed

Lines changed: 25 additions & 25 deletions

File tree

dojo/utils.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
from django.shortcuts import redirect as django_redirect
5252
from django.urls import get_resolver, reverse
5353
from django.utils import timezone
54+
from django.utils.html import escape
5455
from django.utils.http import url_has_allowed_host_and_scheme
5556
from django.utils.translation import gettext as _
5657
from kombu import Connection
@@ -1532,13 +1533,8 @@ def get_current_request():
15321533

15331534

15341535
def create_bleached_link(url, title):
1535-
link = '<a href="'
1536-
link += url
1537-
link += '" target="_blank" title="'
1538-
link += title
1539-
link += '">'
1540-
link += title
1541-
link += "</a>"
1536+
# nh3 requires an explicit escape
1537+
link = f'<a href="{escape(url)}" target="_blank" title="{escape(title)}">{escape(title)}</a>'
15421538
return nh3.clean(link, tags={"a"}, attributes={"a": {"href", "target", "title"}})
15431539

15441540

unittests/test_finding_model.py

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def test_get_sast_source_file_path_with_link_and_source_code_management_uri(self
9595
finding.test = test
9696
finding.sast_source_file_path = "SastSourceFilePath"
9797
engagement.source_code_management_uri = "URL"
98-
self.assertEqual('<a href="URL/SastSourceFilePath" target="_blank" title="SastSourceFilePath">SastSourceFilePath</a>', finding.get_sast_source_file_path_with_link())
98+
self.assertEqual('<a href="URL/SastSourceFilePath" target="_blank" title="SastSourceFilePath" rel="noopener noreferrer">SastSourceFilePath</a>', finding.get_sast_source_file_path_with_link())
9999

100100
def test_get_file_path_with_link_no_file_path(self):
101101
finding = Finding()
@@ -118,7 +118,7 @@ def test_get_file_path_with_link_and_source_code_management_uri(self):
118118
finding.test = test
119119
finding.file_path = "FilePath"
120120
engagement.source_code_management_uri = "URL"
121-
self.assertEqual('<a href="URL/FilePath" target="_blank" title="FilePath">FilePath</a>', finding.get_file_path_with_link())
121+
self.assertEqual('<a href="URL/FilePath" target="_blank" title="FilePath" rel="noopener noreferrer">FilePath</a>', finding.get_file_path_with_link())
122122

123123
def test_get_file_path_with_link_and_source_code_management_uri_github_no_scm_type_with_details_and_line(self):
124124
# checks that for github.com in uri dojo makes correct url to browse on github
@@ -133,7 +133,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_github_no_scm_ty
133133
finding.file_path = "some-folder/some-file.ext"
134134
finding.line = 5432
135135
engagement.source_code_management_uri = "https://github.com/some-test-account/some-test-repo"
136-
self.assertEqual('<a href="https://github.com/some-test-account/some-test-repo/blob/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
136+
self.assertEqual('<a href="https://github.com/some-test-account/some-test-repo/blob/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
137137

138138
def test_get_file_path_with_link_and_source_code_management_uri_github_with_scm_type_with_details_and_line(self):
139139
# checks that for github in custom field dojo makes correct url to browse on github
@@ -157,7 +157,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_github_with_scm_
157157
finding.line = 5432
158158

159159
engagement.source_code_management_uri = "https://github.com/some-test-account/some-test-repo"
160-
self.assertEqual('<a href="https://github.com/some-test-account/some-test-repo/blob/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
160+
self.assertEqual('<a href="https://github.com/some-test-account/some-test-repo/blob/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
161161

162162
def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_public_project_with_no_details_and_line(self):
163163
# checks that for public bitbucket (bitbucket.org) in custom field
@@ -180,7 +180,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_public
180180
finding.line = 5432
181181

182182
engagement.source_code_management_uri = "https://bb.example.com/some-test-user/some-test-repo.git"
183-
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/master/some-folder/some-file.ext#lines-5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
183+
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/master/some-folder/some-file.ext#lines-5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
184184

185185
def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_public_project_with_commithash_and_line(self):
186186
# checks that for public bitbucket (bitbucket.org) in custom field and existing commit hash in finding
@@ -204,7 +204,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_public
204204
finding.line = 5432
205205

206206
engagement.source_code_management_uri = "https://bb.example.com/some-test-user/some-test-repo.git"
207-
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/some-commit-hash/some-folder/some-file.ext#lines-5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
207+
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/some-commit-hash/some-folder/some-file.ext#lines-5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
208208

209209
def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standalone_project_with_commithash_and_line(self):
210210
# checks that for standalone bitbucket in custom field and existing commit hash in finding
@@ -228,7 +228,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standa
228228
finding.line = 5432
229229

230230
engagement.source_code_management_uri = "https://bb.example.com/scm/some-test-project/some-test-repo.git"
231-
self.assertEqual('<a href="https://bb.example.com/projects/some-test-project/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-commit-hash#5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
231+
self.assertEqual('<a href="https://bb.example.com/projects/some-test-project/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-commit-hash#5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
232232

233233
def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standalone_project_with_branchtag_and_line(self):
234234
# checks that for standalone bitbucket in custom field and existing branch/tag in finding
@@ -252,7 +252,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standa
252252
finding.line = 5432
253253

254254
engagement.source_code_management_uri = "https://bb.example.com/scm/some-test-project/some-test-repo.git"
255-
self.assertEqual('<a href="https://bb.example.com/projects/some-test-project/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-branch#5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
255+
self.assertEqual('<a href="https://bb.example.com/projects/some-test-project/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-branch#5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
256256

257257
def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standalone_user_with_branchtag_and_line(self):
258258
# checks that for standalone bitbucket in custom field and existing branch/tag in finding
@@ -277,7 +277,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_bitbucket_standa
277277

278278
engagement.source_code_management_uri = "https://bb.example.com/scm/~some-user/some-test-repo.git"
279279

280-
self.assertEqual('<a href="https://bb.example.com/users/some-user/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-branch#5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
280+
self.assertEqual('<a href="https://bb.example.com/users/some-user/repos/some-test-repo/browse/some-folder/some-file.ext?at=some-branch#5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
281281

282282
def test_get_file_path_with_link_and_source_code_management_uri_gitea_or_codeberg_project_with_no_details_and_line(self):
283283
# checks that for gitea and codeberg in custom field
@@ -300,7 +300,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_gitea_or_codeber
300300
finding.line = 5432
301301

302302
engagement.source_code_management_uri = "https://bb.example.com/some-test-user/some-test-repo.git"
303-
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/master/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
303+
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/master/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
304304

305305
def test_get_file_path_with_link_and_source_code_management_uri_gitea_or_codeberg_project_with_commithash_and_line(self):
306306
# checks that for gitea and codeberg in custom field and existing commit hash in finding
@@ -324,7 +324,7 @@ def test_get_file_path_with_link_and_source_code_management_uri_gitea_or_codeber
324324
finding.line = 5432
325325

326326
engagement.source_code_management_uri = "https://bb.example.com/some-test-user/some-test-repo.git"
327-
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
327+
self.assertEqual('<a href="https://bb.example.com/some-test-user/some-test-repo/src/some-commit-hash/some-folder/some-file.ext#L5432" target="_blank" title="some-folder/some-file.ext" rel="noopener noreferrer">some-folder/some-file.ext</a>', finding.get_file_path_with_link())
328328

329329
def test_get_file_path_with_xss_attack(self):
330330
test = Test()
@@ -334,7 +334,11 @@ def test_get_file_path_with_xss_attack(self):
334334
finding.test = test
335335
finding.file_path = "<SCRIPT SRC=http://xss.rocks/xss.js></SCRIPT>"
336336
engagement.source_code_management_uri = "<IMG SRC=javascript:alert('XSS')>"
337-
self.assertEqual('<a href="&lt;IMG SRC=javascript:alert(\'XSS\')>/&lt;SCRIPT SRC=http://xss.rocks/xss.js>&lt;/SCRIPT>" target="_blank" title="&lt;SCRIPT SRC=http://xss.rocks/xss.js>&lt;/SCRIPT>">&lt;SCRIPT SRC=http://xss.rocks/xss.js&gt;&lt;/SCRIPT&gt;</a>', finding.get_file_path_with_link())
337+
result = finding.get_file_path_with_link()
338+
# XSS payloads must be escaped — no raw tags should appear in the output
339+
self.assertNotIn("<SCRIPT", result)
340+
self.assertNotIn("<IMG", result)
341+
self.assertIn('rel="noopener noreferrer"', result)
338342

339343
def test_get_references_with_links_no_references(self):
340344
finding = Finding()
@@ -348,32 +352,32 @@ def test_get_references_with_links_no_links(self):
348352
def test_get_references_with_links_simple_url(self):
349353
finding = Finding()
350354
finding.references = "URL: https://www.example.com"
351-
self.assertEqual('URL: <a href="https://www.example.com" target="_blank" title="https://www.example.com">https://www.example.com</a>', finding.get_references_with_links())
355+
self.assertEqual('URL: <a href="https://www.example.com" target="_blank" title="https://www.example.com" rel="noopener noreferrer">https://www.example.com</a>', finding.get_references_with_links())
352356

353357
def test_get_references_with_links_url_with_port(self):
354358
finding = Finding()
355359
finding.references = "http://www.example.com:8080"
356-
self.assertEqual('<a href="http://www.example.com:8080" target="_blank" title="http://www.example.com:8080">http://www.example.com:8080</a>', finding.get_references_with_links())
360+
self.assertEqual('<a href="http://www.example.com:8080" target="_blank" title="http://www.example.com:8080" rel="noopener noreferrer">http://www.example.com:8080</a>', finding.get_references_with_links())
357361

358362
def test_get_references_with_links_url_with_path(self):
359363
finding = Finding()
360364
finding.references = "URL https://www.example.com/path/part2 behind URL"
361-
self.assertEqual('URL <a href="https://www.example.com/path/part2" target="_blank" title="https://www.example.com/path/part2">https://www.example.com/path/part2</a> behind URL', finding.get_references_with_links())
365+
self.assertEqual('URL <a href="https://www.example.com/path/part2" target="_blank" title="https://www.example.com/path/part2" rel="noopener noreferrer">https://www.example.com/path/part2</a> behind URL', finding.get_references_with_links())
362366

363367
def test_get_references_with_links_complicated_url_with_parameter(self):
364368
finding = Finding()
365369
finding.references = "URL:https://www.example.com/path?param1=abc&_param2=xyz"
366-
self.assertEqual('URL:<a href="https://www.example.com/path?param1=abc&amp;_param2=xyz" target="_blank" title="https://www.example.com/path?param1=abc&amp;_param2=xyz">https://www.example.com/path?param1=abc&amp;_param2=xyz</a>', finding.get_references_with_links())
370+
self.assertEqual('URL:<a href="https://www.example.com/path?param1=abc&amp;_param2=xyz" target="_blank" title="https://www.example.com/path?param1=abc&amp;_param2=xyz" rel="noopener noreferrer">https://www.example.com/path?param1=abc&amp;_param2=xyz</a>', finding.get_references_with_links())
367371

368372
def test_get_references_with_links_two_urls(self):
369373
finding = Finding()
370374
finding.references = "URL1: https://www.example.com URL2: https://info.example.com"
371-
self.assertEqual('URL1: <a href="https://www.example.com" target="_blank" title="https://www.example.com">https://www.example.com</a> URL2: <a href="https://info.example.com" target="_blank" title="https://info.example.com">https://info.example.com</a>', finding.get_references_with_links())
375+
self.assertEqual('URL1: <a href="https://www.example.com" target="_blank" title="https://www.example.com" rel="noopener noreferrer">https://www.example.com</a> URL2: <a href="https://info.example.com" target="_blank" title="https://info.example.com" rel="noopener noreferrer">https://info.example.com</a>', finding.get_references_with_links())
372376

373377
def test_get_references_with_links_linebreak(self):
374378
finding = Finding()
375379
finding.references = "https://www.example.com\nhttps://info.example.com"
376-
self.assertEqual('<a href="https://www.example.com" target="_blank" title="https://www.example.com">https://www.example.com</a>\n<a href="https://info.example.com" target="_blank" title="https://info.example.com">https://info.example.com</a>', finding.get_references_with_links())
380+
self.assertEqual('<a href="https://www.example.com" target="_blank" title="https://www.example.com" rel="noopener noreferrer">https://www.example.com</a>\n<a href="https://info.example.com" target="_blank" title="https://info.example.com" rel="noopener noreferrer">https://info.example.com</a>', finding.get_references_with_links())
377381

378382
def test_get_references_with_links_markdown(self):
379383
finding = Finding()

0 commit comments

Comments
 (0)