Preserve text gates in renamed literal-content elements

change_log.md

@@ -2,6 +2,10 @@
 
 Most recent at top.
   * Next release
+    * A nested element inside one renamed to a literal-content element no
+      longer opens its own text gate.  Its text now follows the outer element's
+      gate, so it cannot reach `style` or similar content where text was not
+      allowed (#482).
     * Docs: Add focused README files for each Maven module and an index for
       supporting documentation, including safe build and regeneration steps.
     * Table parts inside cell content now return to the existing table, and a

owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java

@@ -85,6 +85,12 @@ class ElementAndAttributePolicyBasedSanitizerPolicy
    * without counting them toward its nesting limit.
    */
   transient boolean skipText = true;
+  /**
+   * True while an emitted element whose content the renderer writes literally
+   * is open.  A nested start tag cannot be emitted in that context, so it must
+   * be treated as dropped before it can become a text container in the policy.
+   */
+  private boolean inKeptLiteralElement;
   /**
    * True while a kept element whose text the renderer emits unescaped, such
    * as {@code <style>}, {@code <script>} or {@code <iframe>}, is open as an
@@ -129,6 +135,8 @@ class ElementAndAttributePolicyBasedSanitizerPolicy
    * {@code k} elements restores it.
    */
   private final BitSet skipTextBeforeOpen = new BitSet();
+  /** The same for {@link #inKeptLiteralElement}. */
+  private final BitSet inKeptLiteralBeforeOpen = new BitSet();
   /** The same for {@link #inKeptCdataElement}. */
   private final BitSet inKeptCdataBeforeOpen = new BitSet();
   /** The same for {@link #inForeignContent}. */
@@ -188,6 +196,7 @@ class ElementAndAttributePolicyBasedSanitizerPolicy
 
   public void openDocument() {
     skipText = false;
+    inKeptLiteralElement = false;
     inKeptCdataElement = false;
     keptCdataElementName = null;
     inForeignContent = false;
@@ -199,6 +208,7 @@ public void openDocument() {
     skippedLastTagAsAttributeless = false;
     openElementStack.clear();
     skipTextBeforeOpen.clear();
+    inKeptLiteralBeforeOpen.clear();
     inKeptCdataBeforeOpen.clear();
     inForeignContentBeforeOpen.clear();
     keptCdataNameBeforeOpen.clear();
@@ -215,10 +225,12 @@ public void closeDocument() {
     }
     openElementStack.clear();
     skipTextBeforeOpen.clear();
+    inKeptLiteralBeforeOpen.clear();
     inKeptCdataBeforeOpen.clear();
     inForeignContentBeforeOpen.clear();
     keptCdataNameBeforeOpen.clear();
     skipText = true;
+    inKeptLiteralElement = false;
     inKeptCdataElement = false;
     keptCdataElementName = null;
     inForeignContent = false;
@@ -798,6 +810,11 @@ public void openReopenedTable(List<String> attrs) {
   private void openTag(
       String elementName, List<String> attrs, OpenTagMode mode) {
     outputElementNameForLastOpenTag = null;
+    if (inKeptLiteralElement) {
+      skippedLastTagAsAttributeless = false;
+      deferOpenTag(elementName);
+      return;
+    }
     ElementAndAttributePolicies policies = elAndAttrPolicies.get(elementName);
     String adjustedElementName = applyPolicies(elementName, attrs, policies);
     skippedLastTagAsAttributeless = false;
@@ -902,6 +919,7 @@ public void closeTag(String elementName) {
         }
         openElementStack.subList(i, n).clear();
         skipText = skipTextBeforeOpen.get(i / 2);
+        inKeptLiteralElement = inKeptLiteralBeforeOpen.get(i / 2);
         inKeptCdataElement = inKeptCdataBeforeOpen.get(i / 2);
         inForeignContent = inForeignContentBeforeOpen.get(i / 2);
         keptCdataElementName = keptCdataNameBeforeOpen.get(i / 2);
@@ -962,6 +980,7 @@ void writeOpenTag(
       keptCdataElementName = adjustedElementName;
       literalTextTail = "";
     }
+    inKeptLiteralElement = inKeptLiteralElement || literal;
     inKeptCdataElement = inKeptCdataElement || enteringKeptCdata;
     // Judged before this element is in foreign content itself: a browser
     // parses the content of an svg or math element as markup, but the
@@ -989,6 +1008,7 @@ void deferOpenTag(String elementName) {
   private void push(String elementName, @Nullable String adjustedElementName) {
     int depth = openElementStack.size() / 2;
     skipTextBeforeOpen.set(depth, skipText);
+    inKeptLiteralBeforeOpen.set(depth, inKeptLiteralElement);
     inKeptCdataBeforeOpen.set(depth, inKeptCdataElement);
     inForeignContentBeforeOpen.set(depth, inForeignContent);
     keptCdataNameBeforeOpen.add(keptCdataElementName);

owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlChangeReporterTest.java

@@ -722,8 +722,8 @@ void testAddedAttributeWithAnInvalidNameIsReported() {
 
   /**
    * A tag arriving inside literal content the renderer is writing is dropped
-   * as content that cannot appear there, which a policy that renames an
-   * element into {@code style} brings about.  Reported under the input name.
+   * before its attributes are judged, which a policy that renames an element
+   * into {@code style} brings about.  Reported under the input name.
    */
   @Test
   void testTagInsideRenamedLiteralContentElementIsReported() {
@@ -732,7 +732,8 @@ void testTagInsideRenamedLiteralContentElementIsReported() {
         .allowElements("b")
         .allowTextIn("style")
         .toFactory();
-    Result result = sanitizeVerbose(policy, "<div>a<b>bold</b>c</div>");
+    Result result = sanitizeVerbose(
+        policy, "<div>a<b onclick=alert(1)>bold</b>c</div>");
 
     assertEquals("<style>aboldc</style>", result.html);
     assertEquals("<b> ", result.log);

owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java

@@ -2059,6 +2059,20 @@ void testRenameIntoALiteralContentElementStillNeedsAllowTextInOnTheTarget() {
         apply(divToStyle.allowTextIn("style"), "<div>a{b:c}</div>"));
   }
 
+  /** A nested kept element does not override a literal element's text gate. */
+  @Test
+  void testNestedElementInsideRenamedLiteralContentUsesOuterTextGate() {
+    HtmlPolicyBuilder divToStyle = new HtmlPolicyBuilder()
+        .allowElements((name, attrs) -> "style", "div")
+        .allowElements("b", "style");
+    String html = "<div>a<b>bold</b>c</div><b>after</b>";
+
+    assertEquals("<style></style><b>after</b>", apply(divToStyle, html));
+    assertEquals(
+        "<style>aboldc</style><b>after</b>",
+        apply(divToStyle.allowTextIn("style"), html));
+  }
+
   /**
    * A void element renamed to one that is not void is closed at once (#450).
    * The lexer never produces a close tag for {@code br}, and the balancer,