Skip to content

Commit cddc25c

Browse files
feanilclaude
andcommitted
fix: remove style tags from discussion email notification HTML
`clean_thread_html_body()` was missing `<style>` from its tag denylist, allowing arbitrary CSS to survive sanitization and be rendered via the `|safe` filter in email templates. This enabled CSS-based email tracking (IP disclosure via background-image/import), content spoofing, and phishing via pseudo-elements. Uses `decompose()` rather than `unwrap()` so the CSS text content is also removed, not just the tag wrapper. Ref: GHSA-4xv3-5j4x-q8g4 Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 160e7e6 commit cddc25c

2 files changed

Lines changed: 21 additions & 0 deletions

File tree

lms/djangoapps/discussion/rest_api/discussions_notifications.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,14 @@ def clean_thread_html_body(html_body):
465465
truncated_body = html.unescape(truncated_body)
466466
html_body = BeautifulSoup(truncated_body, 'html.parser')
467467

468+
# Remove tags including their content (decompose, not unwrap)
469+
tags_to_decompose = [
470+
"style", # CSS injection
471+
]
472+
for tag in tags_to_decompose:
473+
for match in html_body.find_all(tag):
474+
match.decompose()
475+
468476
tags_to_remove = [
469477
"a", "link", # Link Tags
470478
"img", "picture", "source", # Image Tags

lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,16 @@ def test_strip_empty_tags(self):
217217
html_body = '<div><p></p><p>content</p><p></p></div>'
218218
result = clean_thread_html_body(html_body)
219219
self.assertEqual(result, '<p style="margin: 0"><p style="margin: 0">content</p></p>') # noqa: PT009
220+
221+
def test_style_tag_removed_with_content(self):
222+
"""
223+
Test that style tags and their CSS content are fully removed (CSS injection prevention).
224+
"""
225+
html_body = (
226+
'<p>Hello</p>'
227+
'<style>body { background-image: url("https://evil.com/track"); }</style>'
228+
)
229+
result = clean_thread_html_body(html_body)
230+
self.assertNotIn('<style>', result) # noqa: PT009
231+
self.assertNotIn('background-image', result) # noqa: PT009
232+
self.assertNotIn('evil.com', result) # noqa: PT009

0 commit comments

Comments
 (0)