From be33ec6962fa3938ec028873e269632d5d8fa55a Mon Sep 17 00:00:00 2001
From: Mike Samuel <mikesamuel@gmail.com>
Date: Mon, 18 Oct 2021 09:23:39 -0400
Subject: [PATCH] Render style tag content more strictly.

This addresses a vulnerability where policies that allow `<style>`
elements with text in `<option>` elements are vulnerable to XSS as
disclosed in

https://docs.google.com/document/d/11SoX296sMS0XoQiQbpxc5pNxSdbJKDJkm5BDv0zrX50/edit?usp=sharing

This changes behavior for rendering of `<style>` element text so may
change behavior.

Specifically, `<style>` element text that includes the strings `-->`
or `]]>` will no longer sanitize.
---
 docs/credits.md                               |  1 +
 docs/cve202142575.md                          |  5 ++
 docs/vulnerabilities.md                       |  3 +-
 .../org/owasp/html/HtmlStreamRenderer.java    | 29 ++++++++++-
 .../java/org/owasp/html/SanitizersTest.java   | 49 ++++++++++++++++++-
 .../TagBalancingHtmlStreamRendererTest.java   |  4 +-
 6 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 docs/cve202142575.md

diff --git a/docs/credits.md b/docs/credits.md
index ed129d43..cad3b048 100644
--- a/docs/credits.md
+++ b/docs/credits.md
@@ -14,6 +14,7 @@
  * rfamilya
  * robinhouston
  * sneha patil
+ * tomanthony
  * vytah
  * willikins_bear
  * yangbongsoo
diff --git a/docs/cve202142575.md b/docs/cve202142575.md
new file mode 100644
index 00000000..241c5180
--- /dev/null
+++ b/docs/cve202142575.md
@@ -0,0 +1,5 @@
+# CVE-2021-42575
+
+Policies that allow `<style>` tags inside `<option>` elements are vulnerable to RCE
+
+See https://docs.google.com/document/d/11SoX296sMS0XoQiQbpxc5pNxSdbJKDJkm5BDv0zrX50/edit?usp=sharing for mitigations.
diff --git a/docs/vulnerabilities.md b/docs/vulnerabilities.md
index 67b34649..80e5fb85 100644
--- a/docs/vulnerabilities.md
+++ b/docs/vulnerabilities.md
@@ -1,3 +1,4 @@
-# Known & public vulnerabilities in this project 
+# Known & public vulnerabilities in this project
 
+  * [CVE-2021-42575](cve202142575.md) - 18 Oct. 2021 - Recommend upgrade to latest version.
   * [CVE-2011-4457](cve20114457.md) - 17 Nov. 2011 - Recommend upgrade to r88 or later.
diff --git a/src/main/java/org/owasp/html/HtmlStreamRenderer.java b/src/main/java/org/owasp/html/HtmlStreamRenderer.java
index 16645531..23d94d06 100644
--- a/src/main/java/org/owasp/html/HtmlStreamRenderer.java
+++ b/src/main/java/org/owasp/html/HtmlStreamRenderer.java
@@ -29,11 +29,15 @@
 package org.owasp.html;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+
 import java.io.Closeable;
 import java.io.Flushable;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 import javax.annotation.WillCloseWhenClosed;
 import javax.annotation.concurrent.NotThreadSafe;
 
@@ -250,7 +254,26 @@ private final void writeCloseTag(String uncanonElementName)
         Encoding.stripBannedCodeunits(cdataContent);
         int problemIndex = checkHtmlCdataCloseable(lastTagOpened, cdataContent);
         if (problemIndex == -1) {
-          output.append(cdataContent);
+          String prefix = "";
+          String suffix = "";
+          Set<String> bannedSubstrings = Collections.emptySet();
+          if ("style".equals(elementName)) {
+            prefix = "/*<![CDATA[<!--*/\n";
+            suffix = "\n/*-->]]>*/";
+            bannedSubstrings = BANNED_IN_STYLE_ELEMENTS;
+          }
+
+          for (String bannedSubstring : bannedSubstrings) {
+            if (cdataContent.indexOf(bannedSubstring) >= 0) {
+              cdataContent.setLength(0);
+            }
+          }
+
+          if (cdataContent.length() != 0) {
+            output.append(prefix);
+            output.append(cdataContent);
+            output.append(suffix);
+          }
         } else {
           error(
               "Invalid CDATA text content",
@@ -434,4 +457,8 @@ public void close() throws IOException {
   private static boolean isTagEnd(char ch) {
     return ch < 63 && 0 != (TAG_ENDS & (1L << ch));
   }
+
+  private static Set<String> BANNED_IN_STYLE_ELEMENTS = ImmutableSet.of(
+    "<![CDATA[", "]]>", "<!--", "-->"
+  );
 }
diff --git a/src/test/java/org/owasp/html/SanitizersTest.java b/src/test/java/org/owasp/html/SanitizersTest.java
index c75fbcb4..db522309 100644
--- a/src/test/java/org/owasp/html/SanitizersTest.java
+++ b/src/test/java/org/owasp/html/SanitizersTest.java
@@ -434,6 +434,53 @@ public static final void testStyleTagInTable() {
         pf.sanitize(input));
   }
 
+  @Test
+  public static final void testStyleTagsInAllTheWrongPlaces() {
+    String input = ""
+      + "<select><option><style><script>alert(1)</script></style></option></select>"
+      + "<svg><style>.r { color: red }</style></svg>"
+      + "<style>.b { color: blue }</style>"
+      + "<style>#a { content: \"<!--\" }</style>"
+      + "<style>#a { content: \"<![CDATA[\" }</style>"
+      + "<style>#a { content: \"-->\" }</style>"
+      + "<style>#a { content: \"]]>\" }</style>";
+    PolicyFactory pf = new HtmlPolicyBuilder()
+        .allowElements("option", "select", "style", "svg")
+        .allowTextIn("style")
+        .toFactory();
+    assertEquals(
+        ""
+        + "<select><option>"
+        + "<style>/*<![CDATA[<!--*/\n<script>alert(1)</script>\n/*-->]]>*/</style>"
+        + "</option></select>"
+        + "<svg>"
+        + "<style>/*<![CDATA[<!--*/\n.r { color: red }\n/*-->]]>*/</style>"
+        + "</svg>"
+        + "<style>/*<![CDATA[<!--*/\n.b { color: blue }\n/*-->]]>*/</style>"
+        + "<style></style>"
+        + "<style></style>"
+        + "<style></style>"
+        + "<style></style>",
+        pf.sanitize(input)
+    );
+  }
+
+  @Test
+  public static final void testSelectIsOdd() {
+    String input = "<select><option><xmp><script>alert(1)</script></xmp></option></select>";
+    PolicyFactory pf = new HtmlPolicyBuilder()
+        .allowElements("option", "select", "xmp")
+        .allowTextIn("xmp")
+        .toFactory();
+    assertEquals(
+        ""
+        + "<select><option>"
+        + "<pre>&lt;script&gt;alert(1)&lt;/script&gt;</pre>"
+        + "</option></select>",
+        pf.sanitize(input)
+    );
+  }
+
   @Test
   public static final void testStyleGlobally() {
     PolicyFactory policyBuilder = new HtmlPolicyBuilder()
@@ -449,7 +496,7 @@ static int fac(int n) {
     int ifac = 1;
     for (int i = 1; i <= n; ++i) {
       int ifacp = ifac * i;
-      if (ifacp < ifac) { throw new IllegalArgumentException("undeflow"); }
+      if (ifacp < ifac) { throw new IllegalArgumentException("underflow"); }
       ifac = ifacp;
     }
     return ifac;
diff --git a/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java b/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java
index 004e95cd..5195fde9 100644
--- a/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java
+++ b/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java
@@ -158,7 +158,9 @@ public final void testTextContent() {
         + "<p>Hello, <textarea>World!</textarea></p>"
         + "<h1>Hello"
         // Text allowed in special style tag.
-        + "<style type=\"text/css\">\n.World {\n  color: blue\n}\n</style></h1>"
+        + "<style type=\"text/css\">/*<![CDATA[<!--*/\n"
+        + "\n.World {\n  color: blue\n}\n"
+        + "\n/*-->]]>*/</style></h1>"
         // Whitespace allowed inside <ul> but non-whitespace text nodes are
         // moved inside <li>.
         + "<ul><li>Hello,</li><li>World!</li></ul>",