1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
|
From 556f750d8d723791fded3476bcd9885d4b97355b Mon Sep 17 00:00:00 2001
From: Andrew Donnellan <ajd@linux.ibm.com>
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 <ajd@linux.ibm.com>
---
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 <ajd@linux.ibm.com>
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 <ajd@linux.ibm.com>
---
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 = 'blah<b>TEST</b>blah'
+ 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('<b>TEST</b>'.encode('utf-8'), response.content)
+
class CommentRedirectTest(TestCase):
--
2.20.1
|
