From 556f750d8d723791fded3476bcd9885d4b97355b Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Mon, 1 Jul 2019 15:28:03 +1000 Subject: [PATCH 1/2] templatetags: Do not mark output of msgid tag as safe The msgid template tag exists to remove angle brackets from either side of the Message-ID header. It also marks its output as safe, meaning it does not get autoescaped by Django templating. Its output is not safe. A maliciously crafted email can include HTML tags inside the Message-ID header, and as long as the angle brackets are not at the start and end of the header, we will quite happily render them. Rather than using mark_safe(), use escape() to explicitly escape the Message-ID. Signed-off-by: Andrew Donnellan --- patchwork/templatetags/patch.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py index ea5a71de362f..757f873b6043 100644 --- a/patchwork/templatetags/patch.py +++ b/patchwork/templatetags/patch.py @@ -5,6 +5,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later from django import template +from django.utils.html import escape from django.utils.safestring import mark_safe from django.template.defaultfilters import stringfilter @@ -64,4 +65,4 @@ def patch_checks(patch): @register.filter @stringfilter def msgid(value): - return mark_safe(value.strip('<>')) + return escape(value.strip('<>')) -- 2.20.1 From 3bf1aa7568a9a1f08f13ed28c5ac6102841bd4dd Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Mon, 1 Jul 2019 18:04:53 +1000 Subject: [PATCH 2/2] tests: Add test for unescaped values in patch detail page Add a test to check whether we are escaping values from the Patch model on the patch detail page. This test shouldn't be relied upon as proof that we've escaped everything correctly, but may help catch regressions. Signed-off-by: Andrew Donnellan --- patchwork/tests/test_detail.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py index 4ca1c9cda2f9..18408ecb95f6 100644 --- a/patchwork/tests/test_detail.py +++ b/patchwork/tests/test_detail.py @@ -34,6 +34,23 @@ class PatchViewTest(TestCase): response = self.client.get(requested_url) self.assertRedirects(response, redirect_url) + def test_escaping(self): + # Warning: this test doesn't guarantee anything - it only tests some + # fields + unescaped_string = 'blahTESTblah' + patch = create_patch() + patch.diff = unescaped_string + patch.commit_ref = unescaped_string + patch.pull_url = unescaped_string + patch.name = unescaped_string + patch.msgid = unescaped_string + patch.headers = unescaped_string + patch.content = unescaped_string + patch.save() + requested_url = reverse('patch-detail', kwargs={'patch_id': patch.id}) + response = self.client.get(requested_url) + self.assertNotIn('TEST'.encode('utf-8'), response.content) + class CommentRedirectTest(TestCase): -- 2.20.1