diff --git a/src/main/java/org/owasp/html/HtmlStreamRenderer.java b/src/main/java/org/owasp/html/HtmlStreamRenderer.java index 23d94d06..9c079d1c 100644 --- a/src/main/java/org/owasp/html/HtmlStreamRenderer.java +++ b/src/main/java/org/owasp/html/HtmlStreamRenderer.java @@ -254,25 +254,8 @@ private final void writeCloseTag(String uncanonElementName) Encoding.stripBannedCodeunits(cdataContent); int problemIndex = checkHtmlCdataCloseable(lastTagOpened, cdataContent); if (problemIndex == -1) { - String prefix = ""; - String suffix = ""; - Set bannedSubstrings = Collections.emptySet(); - if ("style".equals(elementName)) { - prefix = "/*]]>*/"; - 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( @@ -457,8 +440,4 @@ public void close() throws IOException { private static boolean isTagEnd(char ch) { return ch < 63 && 0 != (TAG_ENDS & (1L << ch)); } - - private static Set BANNED_IN_STYLE_ELEMENTS = ImmutableSet.of( - "", "" - ); } diff --git a/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index aa2eb075..6df2a286 100644 --- a/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -217,6 +217,15 @@ && canContain(elIndex, toResume, nOpen)) { private boolean canContain( int child, int container, int containerIndexOnStack) { Preconditions.checkArgument(containerIndexOnStack >= 0); + if (child == HtmlElementTables.TEXT_NODE && hasSpecialTextMode(container)) { + // If there's a select element on the stack, then we need to be extra careful. + int selectElementIndex = METADATA.indexForName("select"); + for (int i = containerIndexOnStack; --i >= 0;) { + if (selectElementIndex == openElements.get(i)) { + return false; + } + } + } int anc = container; int ancIndexOnStack = containerIndexOnStack; while (true) { @@ -363,6 +372,17 @@ private static boolean isHeaderElementName(String canonElementName) { && canonElementName.charAt(1) <= '9'; } + private static boolean hasSpecialTextMode(int elementIndex) { + String name = METADATA.canonNameForIndex(elementIndex); + switch (HtmlTextEscapingMode.getModeForTag(name)) { + case PCDATA: case VOID: + return false; + case CDATA: case CDATA_SOMETIMES: case RCDATA: case PLAIN_TEXT: + return true; + } + throw new IllegalArgumentException(name); + } + private static final byte ALL_SCOPES; private static final byte[] SCOPES_BY_ELEMENT; private static final byte[] SCOPE_FOR_END_TAG; diff --git a/src/test/java/org/owasp/html/SanitizersTest.java b/src/test/java/org/owasp/html/SanitizersTest.java index db522309..4cb7bbca 100644 --- a/src/test/java/org/owasp/html/SanitizersTest.java +++ b/src/test/java/org/owasp/html/SanitizersTest.java @@ -439,11 +439,7 @@ public static final void testStyleTagsInAllTheWrongPlaces() { String input = "" + "" + "" - + "" - + "" - + "" - + "" - + ""; + + ""; PolicyFactory pf = new HtmlPolicyBuilder() .allowElements("option", "select", "style", "svg") .allowTextIn("style") @@ -451,36 +447,49 @@ public static final void testStyleTagsInAllTheWrongPlaces() { assertEquals( "" + "" - + "" + + "<script>alert(1)</script>" + "" + "" - + "" + + "" + "" - + "" - + "" - + "" - + "" - + "", + + "", pf.sanitize(input) ); } @Test public static final void testSelectIsOdd() { + // Special text modes interact badly with select and option String input = ""; PolicyFactory pf = new HtmlPolicyBuilder() .allowElements("option", "select", "xmp") - .allowTextIn("xmp") + .allowTextIn("xmp", "option") .toFactory(); assertEquals( "" - + "" - + "<script>alert(1)</script>" + + "" + + "<script>alert(1)</script>" + "", pf.sanitize(input) ); } + @Test + public static final void testOptionAllowsText() { + String input = "code goes here"; + PolicyFactory pf = new HtmlPolicyBuilder() + .allowElements("option", "select", "pre") + .allowTextIn("pre", "option") + .toFactory(); + assertEquals( + "" + + "" + + "code goes here" + + "", + pf.sanitize(input) + ); + } + @Test public static final void testStyleGlobally() { PolicyFactory policyBuilder = new HtmlPolicyBuilder() diff --git a/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java b/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java index 5195fde9..38a854ef 100644 --- a/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java +++ b/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java @@ -158,9 +158,9 @@ public final void testTextContent() { + "Hello, World!" + "Hello" // Text allowed in special style tag. - + "" + + "" // Whitespace allowed inside but non-whitespace text nodes are // moved inside . + "Hello,World!",
<script>alert(1)</script>
code goes here
Hello, World!