Skip to content

Commit ec2a0f6

Browse files
fix: use escape() directly in create_bleached_link, drop nh3
nh3/ammonia does not re-escape < in attribute values when re-serializing, so passing escape()'d HTML through nh3.clean() still produced raw angle brackets in href/title. The function constructs trusted HTML itself, so nh3 is redundant here — escape() is sufficient and correct. Also adds rel="noopener noreferrer" explicitly and updates tests to match the new output including the exact XSS-escaped form.
1 parent 2a73bf0 commit ec2a0f6

2 files changed

Lines changed: 9 additions & 9 deletions

File tree

dojo/utils.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
import crum
2121
import cvss
22-
import nh3
2322
import redis as redis_lib
2423
import vobject
2524
from amqp.exceptions import ChannelError
@@ -1533,9 +1532,8 @@ def get_current_request():
15331532

15341533

15351534
def create_bleached_link(url, title):
1536-
# nh3 requires an explicit escape
1537-
link = f'<a href="{escape(url)}" target="_blank" title="{escape(title)}">{escape(title)}</a>'
1538-
return nh3.clean(link, tags={"a"}, attributes={"a": {"href", "target", "title"}})
1535+
# escape() encodes text into HTML — the right tool for embedding URLs into attributes, not a sanitizer like nh3
1536+
return f'<a href="{escape(url)}" target="_blank" title="{escape(title)}" rel="noopener noreferrer">{escape(title)}</a>'
15391537

15401538

15411539
def get_object_or_none(klass, *args, **kwargs):

unittests/test_finding_model.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,13 @@ 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-
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)
337+
self.assertEqual(
338+
'<a href="&lt;IMG SRC=javascript:alert(&#x27;XSS&#x27;)&gt;/&lt;SCRIPT SRC=http://xss.rocks/xss.js&gt;&lt;/SCRIPT&gt;"'
339+
' target="_blank"'
340+
' title="&lt;SCRIPT SRC=http://xss.rocks/xss.js&gt;&lt;/SCRIPT&gt;"'
341+
' rel="noopener noreferrer"'
342+
'>&lt;SCRIPT SRC=http://xss.rocks/xss.js&gt;&lt;/SCRIPT&gt;</a>',
343+
finding.get_file_path_with_link())
342344

343345
def test_get_references_with_links_no_references(self):
344346
finding = Finding()

0 commit comments

Comments
 (0)