diff mbox series

[error-report-web,1/2] Don't let Bootstrap insert submitted data as HTML.

Message ID 20261018020518.76802-3-tim.orling@konsulko.com
State New
Headers show
Series Don't render submitted data as HTML | expand

Commit Message

Tim Orling Oct. 18, 2026, 2:03 a.m. UTC 
The filter tooltips used html: true, and their titles hold the
submitted recipe and task, which bleach doesn't clean, so a report could
inject HTML (the CSP stopped scripts). Drop html: true, and build the
commit popover from DOM nodes so the commit is inserted as text.

AI-Generated: Claude Opus 5.5
Signed-off-by: Tim Orling <tim.orling@konsulko.com>
---
 Post/static/js/main.js       | 14 +++++++++++---
 Post/test.py                 | 35 +++++++++++++++++++++++++++++++++++
 templates/latest-errors.html |  7 +------
 3 files changed, 47 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/Post/static/js/main.js b/Post/static/js/main.js
index 5c07ad5..546aabe 100644
--- a/Post/static/js/main.js
+++ b/Post/static/js/main.js
@@ -219,9 +219,16 @@  $(document).ready(function(){
 
   $("#errors-table").show();
 
+  /* Insert the submitted commit as text, not HTML. */
   $('.commit > div').popover({
       placement:'left',
-      html: true
+      html: true,
+      content: function () {
+        var commit = $(this).attr('data-commit');
+        var link = $('<a href="#" class="filter visible" data-type="commit">Filter by commit</a>')
+          .attr('data-filter', commit);
+        return $('<div/>').append($('<span/>').text(commit), $('<p/>').append(link));
+      }
   });
 
   $(".recipe_version a.tooltip-me").tooltip();
@@ -267,8 +274,9 @@  $(document).ready(function(){
     e.stopPropagation();
   });
 
-  $(".icon-filter").tooltip({ container: 'body', html: true });
+  /* Titles hold submitted data: keep them text. */
+  $(".icon-filter").tooltip({ container: 'body' });
   // initialise the remove filter tooltips
-  $("th .btn-mini").tooltip({ container: 'body', html: true, title:'Clear filter' });
+  $("th .btn-mini").tooltip({ container: 'body', title:'Clear filter' });
 
 });
diff --git a/Post/test.py b/Post/test.py
index f48beec..0a358d1 100755
--- a/Post/test.py
+++ b/Post/test.py
@@ -432,6 +432,41 @@  class AccountPagesRemovedTest(TestCase):
         self.assertEqual(client.get("/admin/login/").status_code, 200)
 
 
+@override_settings(ALLOWED_HOSTS=["testhost"])
+class SubmittedHtmlTest(TestCase):
+    """Submitted data must only ever reach the page escaped."""
+
+    def submit(self, **fields):
+        with open("test-data/test-payload.json") as f:
+            payload = json.load(f)
+        payload.update(fields)
+        payload['failures'][0]['package'] = '<b>evil</b>-1.0'
+        payload['failures'][0]['task'] = 'do_<b>evil</b>'
+        client = Client(HTTP_HOST="testhost")
+        data = urllib.parse.urlencode({'data': json.dumps(payload)})
+        self.assertEqual(client.post("/ClientPost/", data, "application/json").status_code, 200)
+        return client, BuildFailure.objects.get()
+
+    def test_failure_fields_are_escaped(self):
+        client, bf = self.submit()
+        response = client.get("/Errors/Latest/")
+        self.assertNotContains(response, "<b>evil</b>")
+        self.assertContains(response, "Filter by &lt;b&gt;evil&lt;/b&gt;: do_&lt;b&gt;evil&lt;/b&gt;")
+
+    def test_commit_is_not_in_html_attributes(self):
+        client, bf = self.submit(branch_commit="master: <b>evil</b>")
+        response = client.get("/Errors/Latest/")
+        self.assertNotContains(response, "<b>evil</b>")
+        self.assertContains(response, 'data-commit="&lt;b&gt;evil&lt;/b&gt;"')
+        # With html enabled, Bootstrap inserts these as HTML.
+        self.assertNotContains(response, "data-content")
+        self.assertNotContains(response, "data-html")
+
+    def test_tooltips_are_not_html(self):
+        with open("Post/static/js/main.js") as f:
+            self.assertNotRegex(f.read(), r"tooltip\(\{[^}]*html: true")
+
+
 @patch("Post.management.commands.culldb.time.sleep")
 class CullDbTest(TestCase):
     def setUp(self):
diff --git a/templates/latest-errors.html b/templates/latest-errors.html
index 2080210..d9c2890 100644
--- a/templates/latest-errors.html
+++ b/templates/latest-errors.html
@@ -189,12 +189,7 @@ 
 
                 </td>
                 <td class="commit">
-                  <div class="btn" rel="popover"
-                      data-content='
-                      {{ build_fail.BUILD.COMMIT|escape}}
-                      <p><a href="#" class="filter visible" data-filter="{{build_fail.BUILD.COMMIT}}" data-type="commit" >Filter by commit</a>'
-                      data-html="true"
-                    >
+                  <div class="btn" rel="popover" data-commit="{{ build_fail.BUILD.COMMIT }}">
                     {{ build_fail.BUILD.COMMIT|truncatechars:10}}
 
                   </div>