Skip to content

Commit db5cf0f

Browse files
committed
Fix XSS in Special Appointment Banner
Special appointment details are provided by the user and as such we need to treat that input as untrusted. This commit ensures that any HTML in the details is properly escaped before being rendered.
1 parent 02d167b commit db5cf0f

2 files changed

Lines changed: 52 additions & 6 deletions

File tree

manage_breast_screening/mammograms/jinja2/mammograms/special_appointments/special_appointment_banner.jinja

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,19 @@
66
{% set rows = [] %}
77
{% for reason in presented_special_appointment.reasons %}
88
{# Build the value content (details) #}
9-
{% set value_content = "" %}
9+
{% set value = {} %}
1010
{% if reason.details %}
11-
{% set value_content = reason.details %}
11+
{% set value = {"html": reason.details | e} %}
1212
{% else %}
13-
{% set value_content = "<span class=\"nhsuk-u-secondary-text-colour\">No details provided</span>" %}
13+
{% set value = {"html": "<span class=\"nhsuk-u-secondary-text-colour\">No details provided</span>"} %}
1414
{% endif %}
1515

1616
{# Add row for this support reason #}
1717
{% do rows.append({
1818
"key": {
1919
"text": reason.label
2020
},
21-
"value": {
22-
"html": value_content | safe
23-
}
21+
"value": value
2422
}) %}
2523
{% endfor %}
2624
{{ summaryList({

manage_breast_screening/mammograms/tests/jinja2/test_special_appointment_banner.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,51 @@ def test_special_appointment_banner_without_change_link(jinja_env, presenter):
112112
</div>
113113
""",
114114
)
115+
116+
117+
def test_special_appointment_banner_escapes_malicious_details(jinja_env):
118+
malicious_details = "<script>alert(1)</script>"
119+
presenter = SpecialAppointmentPresenter(
120+
{
121+
"PHYSICAL_RESTRICTION": {
122+
"details": malicious_details,
123+
}
124+
},
125+
"68d758d0-792d-430f-9c52-1e7a0c2aa1dd",
126+
)
127+
128+
template = jinja_env.from_string(
129+
dedent(
130+
r"""
131+
{% from 'mammograms/special_appointments/special_appointment_banner.jinja' import special_appointment_banner %}
132+
{{ special_appointment_banner(presenter, show_change_link=False) }}
133+
"""
134+
)
135+
)
136+
137+
response = template.render({"presenter": presenter})
138+
139+
assertHTMLEqual(
140+
response,
141+
"""
142+
<div class="nhsuk-card nhsuk-card--warning">
143+
<div class="nhsuk-card__content">
144+
<h3 class="nhsuk-card__heading"><span role="text">
145+
<span class="nhsuk-u-visually-hidden">Important: </span>
146+
Special appointment
147+
</span></h3>
148+
149+
<dl class="nhsuk-summary-list app-special-appointment-banner">
150+
<div class="nhsuk-summary-list__row">
151+
<dt class="nhsuk-summary-list__key">
152+
Physical restriction
153+
</dt>
154+
<dd class="nhsuk-summary-list__value">
155+
&lt;script&gt;alert(1)&lt;/script&gt;
156+
</dd>
157+
</div>
158+
</dl>
159+
</div>
160+
</div>
161+
""",
162+
)

0 commit comments

Comments
 (0)