Skip to content

Commit 6f5cc7b

Browse files
Merge commit from fork
[security] Always sanitize RichText output for safe-HTML output type
2 parents 2b87373 + 781d517 commit 6f5cc7b

3 files changed

Lines changed: 67 additions & 3 deletions

File tree

news/+b4bb1c80.bugfix.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Security: Always sanitize RichText output for safe-HTML output type.
2+
See https://github.com/plone/plone.app.textfield/security/advisories/GHSA-4r4f-gg25-rmg5
3+
@gyst

plone/app/textfield/tests.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,39 @@ def testTransformNone(self):
4747
self.assertEqual(None, value.raw)
4848
self.assertEqual(None, value.output)
4949

50+
def assertXSSPayloadIsSanitized(self, value):
51+
payload = '<img src=x onerror="alert(1)">'
52+
output = value.output_relative_to(self.portal)
53+
54+
self.assertNotIn(payload, output)
55+
self.assertNotIn("onerror", output.lower())
56+
57+
def testOutputSanitizesLazyRichTextValueConstructor(self):
58+
from plone.app.textfield.value import RichTextValue
59+
60+
payload = '<img src=x onerror="alert(1)">'
61+
value = RichTextValue(payload)
62+
63+
self.assertXSSPayloadIsSanitized(value)
64+
65+
def testOutputSanitizesSpoofedSafeHtmlMimeType(self):
66+
from plone.app.textfield.value import RichTextValue
67+
68+
# A value whose input mimeType claims to already be the safe-HTML
69+
# output type must NOT be trusted as pre-sanitized: output must still
70+
# run safe_html. This is the same bypass an untrusted REST client
71+
# reaches by sending content-type: text/x-html-safe (covered by a
72+
# deserializer test in plone.restapi); here we exercise the value
73+
# layer directly to avoid a test-time dependency on plone.restapi.
74+
payload = '<img src=x onerror="alert(1)">'
75+
value = RichTextValue(
76+
payload,
77+
mimeType="text/x-html-safe",
78+
outputMimeType="text/x-html-safe",
79+
)
80+
81+
self.assertXSSPayloadIsSanitized(value)
82+
5083
def testTransformStructured(self):
5184
from plone.app.textfield import RichText
5285
from zope.interface import Interface

plone/app/textfield/value.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99

1010
LOG = logging.getLogger("plone.app.textfield")
1111

12+
HTML_MIME_TYPE = "text/html"
13+
SAFE_HTML_MIME_TYPE = "text/x-html-safe"
14+
1215

1316
class RawValueHolder(Persistent):
1417
"""Place the raw value in a separate persistent object so that it does not
@@ -84,6 +87,14 @@ def mimeType(self):
8487
def outputMimeType(self):
8588
return self._outputMimeType
8689

90+
def _clone_for_transform(self, mimeType, outputMimeType):
91+
clone = self.__class__.__new__(self.__class__)
92+
clone._raw_holder = self._raw_holder
93+
clone._mimeType = mimeType
94+
clone._outputMimeType = outputMimeType
95+
clone._encoding = self.encoding
96+
return clone
97+
8798
@property
8899
def output(self):
89100
site = getSite()
@@ -93,7 +104,7 @@ def output_relative_to(self, context):
93104
"""Transforms the raw value to the output mimetype, within a specified context.
94105
95106
If the value's mimetype is already the same as the output mimetype,
96-
no transformation is performed.
107+
no transformation is performed unless safe HTML output is requested.
97108
98109
The context parameter is relevant when the transformation is
99110
context-dependent. For example, Plone's resolveuid-and-caption
@@ -103,17 +114,34 @@ def output_relative_to(self, context):
103114
If a transformer cannot be found for the specified context, a
104115
transformer with the site as a context is used instead.
105116
"""
106-
if self.mimeType == self.outputMimeType:
117+
if self.raw is None:
118+
return None
119+
120+
mimeType = self.mimeType or HTML_MIME_TYPE
121+
outputMimeType = self.outputMimeType or SAFE_HTML_MIME_TYPE
122+
123+
if mimeType == outputMimeType and outputMimeType != SAFE_HTML_MIME_TYPE:
107124
return self.raw
108125

126+
if mimeType == SAFE_HTML_MIME_TYPE and outputMimeType == SAFE_HTML_MIME_TYPE:
127+
# Treat stored safe HTML as HTML when rendering to the safe HTML
128+
# output type. The safe_html transform is idempotent for already
129+
# safe markup, and this avoids trusting attacker-controlled input
130+
# that merely claims to be text/x-html-safe.
131+
mimeType = HTML_MIME_TYPE
132+
109133
transformer = ITransformer(context, None)
110134
if transformer is None:
111135
site = getSite()
112136
transformer = ITransformer(site, None)
113137
if transformer is None:
114138
return None
115139

116-
return transformer(self, self.outputMimeType)
140+
value = self
141+
if mimeType != self.mimeType or outputMimeType != self.outputMimeType:
142+
value = self._clone_for_transform(mimeType, outputMimeType)
143+
144+
return transformer(value, outputMimeType)
117145

118146
def __repr__(self):
119147
return (

0 commit comments

Comments
 (0)