Escape user-supplied titles reaching fast_html attributes (#888)

src/blog/jinja2/blog/post_preview.jinja2

@@ -4,7 +4,7 @@
             <a href="{{ url('post_detail', args=[post.pk]) }}">{{ post.title }}</a>
         </h3>
         <p>
-            {{ post.excerpt[:PREVIEW_SIZE] | safe }}... <a href="{{ url('post_detail', args=[post.pk]) }}">{{ _("Read More") }}</a>
+            {{ post.excerpt[:PREVIEW_SIZE] }}... <a href="{{ url('post_detail', args=[post.pk]) }}">{{ _("Read More") }}</a>
         </p>
     </div>
 {% endfor %}

src/core/models.py

@@ -6,6 +6,7 @@
 from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save
 from django.dispatch import receiver
 from django.urls import reverse
+from django.utils.html import escape
 from django.utils.translation import gettext_lazy as _
 from django_extensions.db.models import TimeStampedModel
 from fast_html import a, audio, img, render, video
@@ -120,7 +121,7 @@ def used_in_html_string(self):
     def as_html(self):
         attributes = {
             "id": self.id,
-            "title": self.title,
+            "title": escape(self.title),
             "src": self.file.url,
         }
         return render(
@@ -173,7 +174,7 @@ def as_html(
         src = image.url + f"?v={int(self.modified.timestamp())}"
         attributes = {
             "id": self.id,
-            "title": self.title,
+            "title": escape(self.title),
             "src": src,
         }
         return render(
@@ -390,7 +391,7 @@ def is_3d(self):
     def as_html(self, height: int = None, width: int = None):
         attributes = {
             "id": self.id,
-            "title": self.title,
+            "title": escape(self.title),
             "src": self.source.url,
         }
         max_w = width if width else DEFAULT_OBJECT_PREVIEW_WIDTH

src/core/tests/test_html_escaping.py

@@ -0,0 +1,51 @@
+"""Guard the escaping of user-supplied text rendered through fast_html.
+
+`fast_html` escapes nothing, and the modal templates render its output with
+`| safe`, so anything reaching an attribute unescaped is a stored XSS. See #888.
+"""
+
+from django.test import TestCase
+
+from core.tests.factory import MarkerFactory, ObjectFactory, SoundFactory
+
+BREAKOUT = '" onerror="alert(1)'
+
+
+class TestFastHtmlEscaping(TestCase):
+    def test_fast_html_still_does_not_escape(self):
+        """If this ever fails, fast_html started escaping and the call-site
+        escaping below could be reconsidered. Until then it is load-bearing."""
+        from fast_html import img, render
+
+        assert render(img(src="x", title=BREAKOUT)) == (
+            f'<img src="x" title="{BREAKOUT}">'
+        )
+
+    def test_a_marker_title_cannot_break_out_of_the_attribute(self):
+        marker = MarkerFactory(title=BREAKOUT)
+
+        html = marker.as_html()
+
+        assert 'onerror="alert(1)"' not in html
+        assert "&quot;" in html or "&#34;" in html, html
+
+    def test_an_object_title_cannot_break_out_of_the_attribute(self):
+        obj = ObjectFactory(title=BREAKOUT)
+
+        html = obj.as_html()
+
+        assert 'onerror="alert(1)"' not in html
+
+    def test_a_sound_title_cannot_break_out_of_the_attribute(self):
+        sound = SoundFactory(title=BREAKOUT)
+
+        html = sound.as_html()
+
+        assert 'onerror="alert(1)"' not in html
+
+    def test_a_script_tag_in_a_title_is_neutralised(self):
+        marker = MarkerFactory(title="<script>alert(1)</script>")
+
+        html = marker.as_html()
+
+        assert "<script>" not in html