Skip to content

Commit 5eae6e5

Browse files
committed
fix: escape user input in notification content templates
Discussion thread titles (and other user-controlled context fields — replier_name, author_name, username) were interpolated raw into notification.content via `str.format(**context)`. That output is rendered with Django's `|safe` filter in digest_content.html, which is included by both the email_digest and batched_email body templates, so a `<style>` block in a thread title survived into recipient inboxes as executable CSS on email open — enabling open-tracking, content spoofing, and phishing. Escape at the source: in `get_notification_content`, wrap every context value with `django.utils.html.escape` before `template.format(**context)`, exempting the two structural keys (`p`, `strong`) that content_templates use as HTML tag names. This defends every renderer of `notification.content` in one place. This is the incomplete-patch companion of GHSA-4xv3-5j4x-q8g4 (CVE-2026-42857), which sanitized the post body via `clean_thread_html_body()` but did not cover the title path. Fixes GHSA-rv5w-f4r5-h77g.
1 parent 7c5c1dc commit 5eae6e5

2 files changed

Lines changed: 57 additions & 2 deletions

File tree

openedx/core/djangoapps/notifications/base_notification.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44
from typing import Any, Literal, NotRequired, TypedDict
55

6+
from django.utils.html import escape
67
from django.utils.translation import gettext_lazy as _
78

89
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
@@ -14,6 +15,13 @@
1415

1516
FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role'
1617

18+
# Context keys whose values are used as HTML tag names by content_templates
19+
# (e.g. `<{p}>...<{strong}>{post_title}</{strong}></{p}>`). These must NOT be
20+
# HTML-escaped before `template.format(**context)`; every other context value
21+
# must be, since it typically comes from user input (thread title, username,
22+
# etc.). See get_notification_content below.
23+
_STRUCTURAL_CONTEXT_KEYS = frozenset({'p', 'strong'})
24+
1725

1826
class NotificationType(TypedDict):
1927
"""
@@ -422,8 +430,16 @@ def get_notification_content(notification_type: str, context: dict[str, Any]):
422430
context = context_function(context)
423431

424432
if template:
425-
# Handle grouped templates differently by modifying the context using a different function.
426-
return template.format(**context)
433+
# HTML-escape every context value except the structural tag-name
434+
# keys, so that user-controlled input (post_title, replier_name,
435+
# etc.) cannot inject `<style>` / `<script>` / other HTML into
436+
# notification.content — which is rendered with `|safe` in the
437+
# digest and batched email templates.
438+
safe_context = {
439+
key: value if key in _STRUCTURAL_CONTEXT_KEYS else escape(value)
440+
for key, value in context.items()
441+
}
442+
return template.format(**safe_context)
427443

428444
return ''
429445

openedx/core/djangoapps/notifications/tests/test_base_notification.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"""
22
Tests for base_notification
33
"""
4+
import pytest
5+
46
from openedx.core.djangoapps.notifications import base_notification
57
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
68

@@ -62,3 +64,40 @@ def test_validate_non_core_notification_types(self):
6264
assert isinstance(notification_type[key], str)
6365
for key in bool_keys:
6466
assert isinstance(notification_type[key], bool)
67+
68+
69+
@pytest.mark.parametrize(
70+
('user_input', 'escaped'),
71+
[
72+
('<style>body{background:red}</style>evil', '&lt;style&gt;body{background:red}&lt;/style&gt;evil'),
73+
('<script>alert(1)</script>', '&lt;script&gt;alert(1)&lt;/script&gt;'),
74+
('AT&T "quoted"', 'AT&amp;T &quot;quoted&quot;'),
75+
],
76+
)
77+
def test_get_notification_content_escapes_user_input(user_input, escaped):
78+
"""
79+
Regression test for GHSA-rv5w-f4r5-h77g: user-controlled context values
80+
must be HTML-escaped before being interpolated into a content_template
81+
via `str.format`. Structural context keys (`p`, `strong`) are exempt so
82+
the template can still emit real <p>/<strong> tags.
83+
"""
84+
context = {'replier_name': 'alice', 'post_title': user_input}
85+
content = base_notification.get_notification_content('new_response', context)
86+
assert '<style>' not in content
87+
assert '<script>' not in content
88+
assert escaped in content
89+
90+
91+
def test_get_notification_content_preserves_structural_tags():
92+
"""
93+
Companion to test_get_notification_content_escapes_user_input: verify
94+
that the structural `p` and `strong` keys still produce real HTML tags
95+
after the escape pass, and that innocuous user input renders as plain
96+
text alongside them.
97+
"""
98+
context = {'replier_name': 'alice', 'post_title': 'Hello world'}
99+
content = base_notification.get_notification_content('new_response', context)
100+
assert '<p>' in content
101+
assert '</p>' in content
102+
assert '<strong>alice</strong>' in content
103+
assert '<strong>Hello world</strong>' in content

0 commit comments

Comments
 (0)