From db5cf0f0ea24a0656a1e004f34fdc3740942fa0f Mon Sep 17 00:00:00 2001 From: James Abley Date: Tue, 28 Apr 2026 18:22:07 +0100 Subject: [PATCH] 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. --- .../special_appointment_banner.jinja | 10 ++-- .../jinja2/test_special_appointment_banner.py | 48 +++++++++++++++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/manage_breast_screening/mammograms/jinja2/mammograms/special_appointments/special_appointment_banner.jinja b/manage_breast_screening/mammograms/jinja2/mammograms/special_appointments/special_appointment_banner.jinja index 274529daa..946b319d7 100644 --- a/manage_breast_screening/mammograms/jinja2/mammograms/special_appointments/special_appointment_banner.jinja +++ b/manage_breast_screening/mammograms/jinja2/mammograms/special_appointments/special_appointment_banner.jinja @@ -6,11 +6,11 @@ {% set rows = [] %} {% for reason in presented_special_appointment.reasons %} {# Build the value content (details) #} - {% set value_content = "" %} + {% set value = {} %} {% if reason.details %} - {% set value_content = reason.details %} + {% set value = {"html": reason.details | e} %} {% else %} - {% set value_content = "No details provided" %} + {% set value = {"html": "No details provided"} %} {% endif %} {# Add row for this support reason #} @@ -18,9 +18,7 @@ "key": { "text": reason.label }, - "value": { - "html": value_content | safe - } + "value": value }) %} {% endfor %} {{ summaryList({ diff --git a/manage_breast_screening/mammograms/tests/jinja2/test_special_appointment_banner.py b/manage_breast_screening/mammograms/tests/jinja2/test_special_appointment_banner.py index 6e559ada1..7c22236c1 100644 --- a/manage_breast_screening/mammograms/tests/jinja2/test_special_appointment_banner.py +++ b/manage_breast_screening/mammograms/tests/jinja2/test_special_appointment_banner.py @@ -112,3 +112,51 @@ def test_special_appointment_banner_without_change_link(jinja_env, presenter): """, ) + + +def test_special_appointment_banner_escapes_malicious_details(jinja_env): + malicious_details = "" + presenter = SpecialAppointmentPresenter( + { + "PHYSICAL_RESTRICTION": { + "details": malicious_details, + } + }, + "68d758d0-792d-430f-9c52-1e7a0c2aa1dd", + ) + + template = jinja_env.from_string( + dedent( + r""" + {% from 'mammograms/special_appointments/special_appointment_banner.jinja' import special_appointment_banner %} + {{ special_appointment_banner(presenter, show_change_link=False) }} + """ + ) + ) + + response = template.render({"presenter": presenter}) + + assertHTMLEqual( + response, + """ +
+
+

+ Important: + Special appointment +

+ +
+
+
+ Physical restriction +
+
+ <script>alert(1)</script> +
+
+
+
+
+ """, + )