Skip to content

Commit 0d079cb

Browse files
authored
Merge pull request #1390 from NHSDigital/feature/fix-xss
Fix XSS in Special Appointment Banner
2 parents 50a5f0d + db5cf0f commit 0d079cb

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)