[security] Always sanitize RichText output for safe-HTML output type

Stored XSS: RichTextValue.output returned the raw, unsanitized stored
value whenever mimeType == outputMimeType, skipping the safe_html
transform. Because the safe-HTML output type (text/x-html-safe) is the
type that signifies "already sanitized", any value whose stored mimeType
equals it bypassed sanitization entirely on render. The safe_html
transform itself is sound (it strips on* event-handler attributes and
javascript:/data: URIs); the defect is that the transform was never
invoked for these values.

Two reproduction entrypoints reach this skip with attacker-controlled
input:

  A. Lazy constructor. RichTextValue("<img src=x onerror=alert(1)>")
     defaults both mimeType and outputMimeType to None. None == None
     satisfies the shortcut, so .output returns the raw payload. A
     developer doing the obvious thing to set a RichText field
     programmatically silently disables sanitization.

  B. REST deserialization. A client POSTs a RichText field as
     {"data": "<img src=x onerror=alert(1)>",
      "content-type": "text/x-html-safe"}. The deserializer trusts the
     client-supplied content-type and constructs a RichTextValue whose
     mimeType equals outputMimeType, again hitting the skip. No
     developer code is involved; the framework builds the unsanitized
     value from the request.

The stored value is later rendered with tal:content="structure ...",
which performs no escaping or sanitization, so the payload executes in
the victim's browser. Any user who can set a RichText field (or reach
the REST endpoint) can store a payload that fires for every viewer of
the rendered content.

Fix: in RichTextValue.output_relative_to, when the (effective) output
type is the safe-HTML type, do not honor the mimeType == outputMimeType
shortcut. Instead treat the input as text/html and run the safe_html
transform. The transform is idempotent for genuinely-safe markup, so
legitimate already-sanitized content is unaffected, while
attacker-controlled input that merely claims to be text/x-html-safe is
sanitized. The no-op shortcut is preserved for equal non-safe mimetypes
(e.g. text/plain -> text/plain). None raw values short-circuit to None.

Adds regression tests for both entrypoints: the lazy constructor
(defaulted None mimetypes) and a value whose input mimeType spoofs the
safe-HTML output type. Both assert the onerror payload is absent from
the rendered output. The REST-deserializer entrypoint is covered by a
companion test in plone.restapi, which avoids a test-time dependency on
plone.

Author: gyst

src/plone/app/textfield/tests.py

@@ -47,6 +47,39 @@ def testTransformNone(self):
         self.assertEqual(None, value.raw)
         self.assertEqual(None, value.output)
 
+    def assertXSSPayloadIsSanitized(self, value):
+        payload = '<img src=x onerror="alert(1)">'
+        output = value.output_relative_to(self.portal)
+
+        self.assertNotIn(payload, output)
+        self.assertNotIn("onerror", output.lower())
+
+    def testOutputSanitizesLazyRichTextValueConstructor(self):
+        from plone.app.textfield.value import RichTextValue
+
+        payload = '<img src=x onerror="alert(1)">'
+        value = RichTextValue(payload)
+
+        self.assertXSSPayloadIsSanitized(value)
+
+    def testOutputSanitizesSpoofedSafeHtmlMimeType(self):
+        from plone.app.textfield.value import RichTextValue
+
+        # A value whose input mimeType claims to already be the safe-HTML
+        # output type must NOT be trusted as pre-sanitized: output must still
+        # run safe_html. This is the same bypass an untrusted REST client
+        # reaches by sending content-type: text/x-html-safe (covered by a
+        # deserializer test in plone.restapi); here we exercise the value
+        # layer directly to avoid a test-time dependency on plone.restapi.
+        payload = '<img src=x onerror="alert(1)">'
+        value = RichTextValue(
+            payload,
+            mimeType="text/x-html-safe",
+            outputMimeType="text/x-html-safe",
+        )
+
+        self.assertXSSPayloadIsSanitized(value)
+
     def testTransformStructured(self):
         from plone.app.textfield import RichText
         from zope.interface import Interface

src/plone/app/textfield/value.py

@@ -9,6 +9,9 @@
 
 LOG = logging.getLogger("plone.app.textfield")
 
+HTML_MIME_TYPE = "text/html"
+SAFE_HTML_MIME_TYPE = "text/x-html-safe"
+
 
 class RawValueHolder(Persistent):
     """Place the raw value in a separate persistent object so that it does not
@@ -84,6 +87,14 @@ def mimeType(self):
     def outputMimeType(self):
         return self._outputMimeType
 
+    def _clone_for_transform(self, mimeType, outputMimeType):
+        clone = self.__class__.__new__(self.__class__)
+        clone._raw_holder = self._raw_holder
+        clone._mimeType = mimeType
+        clone._outputMimeType = outputMimeType
+        clone._encoding = self.encoding
+        return clone
+
     @property
     def output(self):
         site = getSite()
@@ -93,7 +104,7 @@ def output_relative_to(self, context):
         """Transforms the raw value to the output mimetype, within a specified context.
 
         If the value's mimetype is already the same as the output mimetype,
-        no transformation is performed.
+        no transformation is performed unless safe HTML output is requested.
 
         The context parameter is relevant when the transformation is
         context-dependent. For example, Plone's resolveuid-and-caption
@@ -103,17 +114,34 @@ def output_relative_to(self, context):
         If a transformer cannot be found for the specified context, a
         transformer with the site as a context is used instead.
         """
-        if self.mimeType == self.outputMimeType:
+        if self.raw is None:
+            return None
+
+        mimeType = self.mimeType or HTML_MIME_TYPE
+        outputMimeType = self.outputMimeType or SAFE_HTML_MIME_TYPE
+
+        if mimeType == outputMimeType and outputMimeType != SAFE_HTML_MIME_TYPE:
             return self.raw
 
+        if mimeType == SAFE_HTML_MIME_TYPE and outputMimeType == SAFE_HTML_MIME_TYPE:
+            # Treat stored safe HTML as HTML when rendering to the safe HTML
+            # output type.  The safe_html transform is idempotent for already
+            # safe markup, and this avoids trusting attacker-controlled input
+            # that merely claims to be text/x-html-safe.
+            mimeType = HTML_MIME_TYPE
+
         transformer = ITransformer(context, None)
         if transformer is None:
             site = getSite()
             transformer = ITransformer(site, None)
             if transformer is None:
                 return None
 
-        return transformer(self, self.outputMimeType)
+        value = self
+        if mimeType != self.mimeType or outputMimeType != self.outputMimeType:
+            value = self._clone_for_transform(mimeType, outputMimeType)
+
+        return transformer(value, outputMimeType)
 
     def __repr__(self):
         return (