This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-xss.git
The following commit(s) were added to refs/heads/master by this push:
new 8232e99 SLING-13334 fix input validation gaps
8232e99 is described below
commit 8232e9956808fccf7007f58936ca6588fe200506
Author: Joerg Hoh <[email protected]>
AuthorDate: Mon Sep 14 17:33:56 2026 +0200
SLING-13334 fix input validation gaps
---
src/main/java/org/apache/sling/xss/XSSAPI.java | 10 +-
.../java/org/apache/sling/xss/impl/XSSAPIImpl.java | 88 +++++++++++--
.../org/apache/sling/xss/impl/XSSFilterImpl.java | 144 +++++++++++++++++++--
.../org/apache/sling/xss/impl/XSSAPIImplTest.java | 140 +++++++++++++++++++-
.../apache/sling/xss/impl/XSSFilterImplTest.java | 75 +++++++++++
5 files changed, 429 insertions(+), 28 deletions(-)
diff --git a/src/main/java/org/apache/sling/xss/XSSAPI.java
b/src/main/java/org/apache/sling/xss/XSSAPI.java
index 8cdd309..0496e5d 100644
--- a/src/main/java/org/apache/sling/xss/XSSAPI.java
+++ b/src/main/java/org/apache/sling/xss/XSSAPI.java
@@ -129,7 +129,10 @@ public interface XSSAPI {
/**
* Validate multi-line comment to be used inside a
<script>...</script> or <style>...</style> block.
Multi-line
- * comment end block is disallowed.
+ * comment end block is disallowed, as are character sequences that would
close the surrounding raw-text element or
+ * change the HTML tokenizer's script-data state (case-insensitive {@code
</script}, {@code </style},
+ * {@code <!--} and {@code -->}), since HTML parsing ends a script
or style element at the first closing tag
+ * regardless of the JavaScript/CSS comment state.
*
* @param comment the comment to be used
* @param defaultComment a default value to use if the comment is
{@code null} or not valid.
@@ -140,6 +143,11 @@ public interface XSSAPI {
/**
* Validate a JSON string
*
+ * <p>The returned string is re-serialized so that it stays inert when
inlined into an HTML
+ * {@code <script>} element: the characters {@code <}, U+2028 and U+2029
are emitted as JSON
+ * unicode escape sequences ({@code \u003C}, {@code \u2028}, {@code
\u2029}),
+ * which represent the same JSON value.</p>
+ *
* @param json the JSON string to validate
* @param defaultJson the default value to use if {@code json} is {@code
null} or not valid
* @return a valid JSON string
diff --git a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
index fef2136..c469fbd 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
@@ -61,6 +61,17 @@ public class XSSAPIImpl implements XSSAPI {
private static final Pattern PATTERN_AUTO_DIMENSION =
Pattern.compile("['\"]?auto['\"]?");
+ /**
+ * Character sequences that must never appear in a multi-line comment that
is embedded inside a
+ * {@code <script>} or {@code <style>} element (the context documented for
+ * {@link org.apache.sling.xss.XSSAPI#getValidMultiLineComment(String,
String)}): besides the
+ * comment-end token itself, HTML raw-text parsing ends such an element at
the first
+ * case-insensitive {@code </script} / {@code </style} regardless of the
JavaScript/CSS comment
+ * state, and {@code <!--} / {@code -->} shift the tokenizer's script-data
escape state.
+ */
+ private static final Pattern PATTERN_MULTI_LINE_COMMENT_BREAKOUT =
+ Pattern.compile("\\*/|</script|</style|<!--|-->",
Pattern.CASE_INSENSITIVE);
+
private SAXParserFactory factory;
private volatile JsonReaderFactory jsonReaderFactory;
@@ -223,22 +234,48 @@ public class XSSAPIImpl implements XSSAPI {
}
private static final String NON_ASCII = "\\x00\\x08\\x0B\\x0C\\x0E-\\x1F";
- /** http://www.w3.org/TR/css-syntax-3/#number-token-diagram */
- private static final String NUMBER =
"[+-]?[\\d]*[\\.]?[\\d]*(?:[e][+-]?\\d+)?";
+ /**
+ * http://www.w3.org/TR/css-syntax-3/#number-token-diagram
+ * <p>
+ * Must not be able to match the empty string and uses possessive
quantifiers so that a
+ * backtracking regex engine can never re-split a digit run between its
sub-expressions: this
+ * production is repeated inside {@link #FUNCTION}, and a nullable,
ambiguous expression inside
+ * a repetition is the classic exponential-backtracking (ReDoS) shape.
+ */
+ private static final String NUMBER =
"[+-]?+(?:\\d++(?:\\.\\d*+)?+|\\.\\d++)(?:e[+-]?+\\d++)?+";
/** http://www.w3.org/TR/css-syntax-3/#hex-digit-diagram */
private static final String HEX_DIGITS = "#[0-9a-f]*";
/** http://www.w3.org/TR/css-syntax-3/#ident-token-diagram */
private static final String IDENTIFIER = "-?[a-z_" + NON_ASCII +
"][\\w_\\-" + NON_ASCII + "]*";
- /** http://www.w3.org/TR/css-syntax-3/#string-token-diagram */
+ /**
+ * http://www.w3.org/TR/css-syntax-3/#string-token-diagram
+ * <p>
+ * Deliberately stricter than the CSS grammar: quote characters of either
kind (raw or
+ * backslash-escaped), backslashes and the markup characters {@code <} and
{@code >} are not
+ * allowed inside the string at all. A validated style token may be
emitted into a single- or
+ * double-quoted {@code style} attribute or into a {@code <style>}
element, and HTML parsing
+ * ignores CSS escaping: a single-quoted CSS string containing a raw
{@code "} (e.g. the token
+ * {@code '" onmouseover="alert(1) '}) would otherwise close a
double-quoted attribute, and
+ * {@code </style>} inside a string would end the style element. The
javascript-scheme guard
+ * tolerates arbitrary embedded whitespace instead of at most one
character.
+ */
private static final String STRING =
-
"\"(?:(?!javascript\\s?:)[^\"^\\\\^\\n]|(?:\\\\\"))*\"|'(?:(?!javascript\\s?:)[^'^\\\\^\\n]|(?:\\\\'))*'";
+
"\"(?:(?!javascript\\s*:)[^\"'^\\\\\\n<>])*\"|'(?:(?!javascript\\s*:)[^\"'^\\\\\\n<>])*'";
/** http://www.w3.org/TR/css-syntax-3/#dimension-token-diagram */
private static final String DIMENSION = NUMBER + IDENTIFIER;
/** http://www.w3.org/TR/css-syntax-3/#percentage-token-diagram */
private static final String PERCENT = NUMBER + "%";
- /** http://www.w3.org/TR/css-syntax-3/#function-token-diagram */
+ /**
+ * http://www.w3.org/TR/css-syntax-3/#function-token-diagram
+ * <p>
+ * Every iteration of the argument loop consumes at least one character
and the quantifiers are
+ * possessive, so matching cost stays linear even for inputs that never
close the parenthesis.
+ * The previous formulation nested nullable, ambiguous alternatives inside
an unbounded
+ * repetition, which let an unterminated token like {@code "z(" + "1" *
45} pin the CPU with
+ * exponential backtracking.
+ */
private static final String FUNCTION =
- IDENTIFIER + "\\((?:(?:" + NUMBER + ")|(?:" + IDENTIFIER +
")|(?:[\\s]*)|(?:,))*\\)";
+ IDENTIFIER + "\\((?:[\\s,]*+(?:(?:" + NUMBER + ")|(?:" +
IDENTIFIER + ")))*+[\\s,]*+\\)";
/** http://www.w3.org/TR/css-syntax-3/#url-unquoted-diagram */
private static final String URL_UNQUOTED = "[^\"^'^\\(^\\)^[" + NON_ASCII
+ "]]*";
/** http://www.w3.org/TR/css-syntax-3/#url-token-diagram */
@@ -295,7 +332,8 @@ public class XSSAPIImpl implements XSSAPI {
*/
@Override
public String getValidMultiLineComment(String comment, String
defaultComment) {
- if (comment != null && !comment.contains("*/")) {
+ if (comment != null
+ &&
!PATTERN_MULTI_LINE_COMMENT_BREAKOUT.matcher(comment).find()) {
return comment;
}
return defaultComment;
@@ -323,7 +361,7 @@ public class XSSAPIImpl implements XSSAPI {
.createReader(new StringReader(json))
.readObject())
.close();
- return output.getBuffer().toString();
+ return escapeJsonForHtmlContext(output.getBuffer().toString());
} catch (Exception e) {
LOGGER.warn("Unable to get valid JSON from the input.", e);
LOGGER.debug("JSON input:\n{}", json);
@@ -336,7 +374,7 @@ public class XSSAPIImpl implements XSSAPI {
.createReader(new StringReader(json))
.readArray())
.close();
- return output.getBuffer().toString();
+ return escapeJsonForHtmlContext(output.getBuffer().toString());
} catch (Exception e) {
LOGGER.warn("Unable to get valid JSON from the input.", e);
LOGGER.debug("JSON input:\n{}", json);
@@ -345,6 +383,38 @@ public class XSSAPIImpl implements XSSAPI {
return getValidJSON(defaultJson, "");
}
+ /**
+ * Escapes the characters that can break out of an HTML {@code <script>}
element or an inline
+ * event handler when serialized JSON is inlined during HTML composition:
{@code <} (which would
+ * otherwise allow {@code </script>} or {@code <!--} sequences inside
string values) and the
+ * JavaScript line terminators U+2028/U+2029. In serialized JSON these
characters can only occur
+ * inside string values, and each replacement is a JSON escape sequence
for the same character,
+ * so the returned string represents exactly the same JSON value.
+ *
+ * @param json a serialized JSON document
+ * @return the equivalent JSON document, safe to inline in HTML script
contexts
+ */
+ private static String escapeJsonForHtmlContext(@NotNull String json) {
+ StringBuilder sb = new StringBuilder(json.length());
+ for (int i = 0; i < json.length(); i++) {
+ char c = json.charAt(i);
+ switch (c) {
+ case '<':
+ sb.append("\\u003C");
+ break;
+ case '\u2028':
+ sb.append("\\u2028");
+ break;
+ case '\u2029':
+ sb.append("\\u2029");
+ break;
+ default:
+ sb.append(c);
+ }
+ }
+ return sb.toString();
+ }
+
/**
* @see org.apache.sling.xss.XSSAPI#getValidXML(String, String)
*/
diff --git a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
index 7bafd38..9e00c2b 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
@@ -22,13 +22,16 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UncheckedIOException;
+import java.io.Writer;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.Dictionary;
+import java.util.HashMap;
import java.util.Hashtable;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -37,7 +40,9 @@ import java.util.regex.Pattern;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.text.StringEscapeUtils;
-import org.apache.commons.text.translate.NumericEntityUnescaper;
+import org.apache.commons.text.translate.AggregateTranslator;
+import org.apache.commons.text.translate.CharSequenceTranslator;
+import org.apache.commons.text.translate.LookupTranslator;
import org.apache.sling.api.resource.LoginException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
@@ -170,11 +175,123 @@ public class XSSFilterImpl implements XSSFilter {
AntiSamyActions.REMOVE_ATTRIBUTE_ON_INVALID,
null);
- /*
- NumericEntityEscaper is deprecated starting with version 3.6 of
commons-lang3, however the indicated replacement comes from
- commons-text, which is not an OSGi bundle
- */
- private static final NumericEntityUnescaper UNICODE_UNESCAPER = new
NumericEntityUnescaper();
+ /**
+ * Unescapes HTML character references in a single left-to-right pass, the
way a browser does when
+ * reading an attribute value: numeric references ({@code &#dd;} / {@code
&#xhh;}, terminating
+ * semicolon optional, e.g. {@code "javascript:alert(1)"} decodes to
{@code "javascript:alert(1)"})
+ * plus named references, both the HTML 4 set known to {@link
StringEscapeUtils#unescapeHtml4(String)}
+ * and the HTML5-only names that resolve to ASCII characters (e.g. {@code
	}, {@code 
},
+ * {@code :}). commons-text's {@code NumericEntityUnescaper} cannot
be used for the numeric part:
+ * it requires the semicolon by default, and its semiColonOptional mode
mis-parses decimal references
+ * followed by a hex-letter character (it reads "ja" as the decimal
number "106a" and gives up).
+ *
+ * <p>Numeric and named decoding are combined into a single {@link
AggregateTranslator} - and applied to
+ * the URL exactly once - rather than run as two sequential passes: a
two-pass approach would let a
+ * character produced by the first pass be mistaken for the start of a new
reference by the second pass.
+ * For example {@code "&Tab;"} is, per the HTML5 spec, decoded once to
the literal text
+ * {@code "	"} (a harmless ampersand followed by literal text); a
browser does not re-scan that
+ * output and would never turn it into a tab character, so this
implementation must not either.
+ */
+ static final CharSequenceTranslator NUMERIC_ENTITY_UNESCAPER = new
Html5NumericEntityUnescaper();
+
+ static final CharSequenceTranslator UNICODE_UNESCAPER = new
AggregateTranslator(
+ NUMERIC_ENTITY_UNESCAPER, StringEscapeUtils.UNESCAPE_HTML4, new
LookupTranslator(html5AsciiEntities()));
+
+ private static Map<CharSequence, CharSequence> html5AsciiEntities() {
+ Map<CharSequence, CharSequence> entities = new HashMap<>();
+ entities.put("	", "\t");
+ entities.put("
", "\n");
+ entities.put("!", "!");
+ entities.put("#", "#");
+ entities.put("$", "$");
+ entities.put("%", "%");
+ entities.put("'", "'");
+ entities.put("(", "(");
+ entities.put(")", ")");
+ entities.put("*", "*");
+ entities.put("*", "*");
+ entities.put("+", "+");
+ entities.put(",", ",");
+ entities.put(".", ".");
+ entities.put("/", "/");
+ entities.put(":", ":");
+ entities.put(";", ";");
+ entities.put("=", "=");
+ entities.put("?", "?");
+ entities.put("@", "@");
+ entities.put("[", "[");
+ entities.put("[", "[");
+ entities.put("\", "\\");
+ entities.put("]", "]");
+ entities.put("]", "]");
+ entities.put("^", "^");
+ entities.put("_", "_");
+ entities.put("_", "_");
+ entities.put("`", "`");
+ entities.put("`", "`");
+ entities.put("{", "{");
+ entities.put("{", "{");
+ entities.put("|", "|");
+ entities.put("|", "|");
+ entities.put("|", "|");
+ entities.put("}", "}");
+ entities.put("}", "}");
+ return Collections.unmodifiableMap(entities);
+ }
+
+ /**
+ * Decodes numeric character references ({@code &#dd;} / {@code &#xhh;})
the way the HTML5
+ * specification requires for attribute values: the terminating semicolon
is optional and a
+ * decimal reference ends at the first non-decimal-digit character (so
{@code javascript}
+ * decodes to {@code javascript}).
+ */
+ private static final class Html5NumericEntityUnescaper extends
CharSequenceTranslator {
+
+ @Override
+ public int translate(CharSequence input, int index, Writer writer)
throws IOException {
+ int seqEnd = input.length();
+ if (input.charAt(index) != '&' || index >= seqEnd - 2 ||
input.charAt(index + 1) != '#') {
+ return 0;
+ }
+ int start = index + 2;
+ boolean isHex = false;
+ char firstChar = input.charAt(start);
+ if (firstChar == 'x' || firstChar == 'X') {
+ start++;
+ isHex = true;
+ if (start == seqEnd) {
+ return 0;
+ }
+ }
+ int end = start;
+ while (end < seqEnd && isEntityDigit(input.charAt(end), isHex)) {
+ end++;
+ }
+ if (end == start) {
+ return 0;
+ }
+ int entityValue;
+ try {
+ entityValue = Integer.parseInt(input.subSequence(start,
end).toString(), isHex ? 16 : 10);
+ } catch (NumberFormatException nfe) {
+ // value too large to represent: decode to the replacement
character, like browsers do
+ entityValue = 0xFFFD;
+ }
+ if (entityValue > Character.MAX_CODE_POINT) {
+ entityValue = 0xFFFD;
+ }
+ writer.write(new String(Character.toChars(entityValue)));
+ boolean semiNext = end != seqEnd && input.charAt(end) == ';';
+ return (semiNext ? end + 1 : end) - index;
+ }
+
+ private static boolean isEntityDigit(char ch, boolean isHex) {
+ if (ch >= '0' && ch <= '9') {
+ return true;
+ }
+ return isHex && (ch >= 'a' && ch <= 'f' || ch >= 'A' && ch <= 'F');
+ }
+ }
// Default href configuration copied from the config.xml supplied with
AntiSamy
static final Attribute DEFAULT_HREF_ATTRIBUTE = new Attribute(
@@ -245,14 +362,13 @@ public class XSSFilterImpl implements XSSFilter {
reportInvalidUrl(url);
return false;
}
- String unicodeUnescapedUrl =
UNICODE_UNESCAPER.translate(decodedURL);
- String urlToValidate;
- if (unicodeUnescapedUrl.equals(decodedURL)) {
- urlToValidate = url;
- } else {
- urlToValidate = unicodeUnescapedUrl;
- }
- urlToValidate = StringEscapeUtils.unescapeHtml4(urlToValidate);
+ String numericUnescapedUrl =
NUMERIC_ENTITY_UNESCAPER.translate(decodedURL);
+ // Decode numeric and named character references in a single pass
over whichever base string is
+ // chosen below: chaining two separate translate() calls would let
a character produced by the
+ // first pass (e.g. the '&' decoded from "&") be mistaken by
the second pass for the start of
+ // a new reference, which a browser never does (see
UNICODE_UNESCAPER's javadoc).
+ String baseUrl = numericUnescapedUrl.equals(decodedURL) ? url :
decodedURL;
+ String urlToValidate = UNICODE_UNESCAPER.translate(baseUrl);
return runHrefValidation(urlToValidate);
} catch (Exception e) {
logger.warn("Unable to validate url.", e);
diff --git a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
index 32d1215..b5d38e7 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.sling.xss.impl;
+import java.time.Duration;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -37,6 +38,7 @@ import org.apache.sling.xss.impl.status.XSSStatusService;
import org.apache.sling.xss.impl.xml.Attribute;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
@@ -48,6 +50,7 @@ import org.osgi.framework.ServiceReference;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.anyString;
@@ -228,6 +231,39 @@ public class XSSAPIImplTest {
}
}
+ @Test
+ public void testGetValidStyleTokenBacktrackingComplexity() {
+ // an unterminated function token used to trigger exponential
backtracking in the FUNCTION
+ // production of CSS_TOKEN (~45 digits were enough to pin a CPU core
practically forever);
+ // validation cost must stay linear in the input length
+ StringBuilder pathological = new
StringBuilder("z(").append("1".repeat(100));
+ String token = pathological.toString();
+ assertTimeoutPreemptively(
+ Duration.ofSeconds(5), () -> assertEquals(RUBBISH,
xssAPI.getValidStyleToken(token, RUBBISH)));
+ }
+
+ static String[] dataForStyleTokenBacktrackingComplexity() {
+ return new String[] {
+ // digit run seasoned with '.' and a trailing exponent marker:
stresses NUMBER's internal
+ // split points between its integer, fractional and exponent parts
+ "z(" + "1".repeat(50) + "." + "1".repeat(50) + "e",
+ // identifier filler: stresses the NUMBER/IDENTIFIER alternation
inside FUNCTION's loop
+ "z(" + "a".repeat(100),
+ // whitespace/comma filler: stresses the outer [\s,]*+ possessive
class
+ "z(" + " ,".repeat(100)
+ };
+ }
+
+ @ParameterizedTest
+ @MethodSource("dataForStyleTokenBacktrackingComplexity")
+ public void
testGetValidStyleTokenBacktrackingComplexityOtherFillerShapes(String token) {
+ // same linear-cost requirement as
testGetValidStyleTokenBacktrackingComplexity, but for other
+ // filler shapes (not just a plain digit run), to guard against a
partial regression that
+ // only hardens one alternative
+ assertTimeoutPreemptively(
+ Duration.ofSeconds(5), () -> assertEquals(RUBBISH,
xssAPI.getValidStyleToken(token, RUBBISH)));
+ }
+
@ParameterizedTest
@MethodSource("dataForValidCSSColor")
public void testGetValidCSSColor(String source, String expected) {
@@ -450,6 +486,15 @@ public class XSSAPIImplTest {
"java&Tab;script:void(document.body.dataset.executed=1)",
"java&Tab;script:void(document.body.dataset.executed=1)"
},
+
+ // HTML5-only named character references and semicolon-less
numeric references are
+ // decoded by browsers before URL parsing and must not smuggle a
javascript: scheme
+ {"java	script:alert(1)", ""},
+ {"java
script:alert(1)", ""},
+ {"javascript:alert(1)", ""},
+ {"java	script:alert(1)", ""},
+ {"javascript:alert(1)", ""},
+ {"javascript:alert(1)", ""},
{"http://localhost:4502";, "http://localhost:4502"},
{"http://localhost:4502/test";, "http://localhost:4502/test"},
{"http://localhost:4502/jcr:content/test";,
"http://localhost:4502/jcr:content/test"},
@@ -659,8 +704,31 @@ public class XSSAPIImplTest {
// valid strings
{"'literal string'", "'literal string'"},
{"\"literal string\"", "\"literal string\""},
- {"'it\\'s here'", "'it\\'s here'"},
- {"\"it\\\"s here\"", "\"it\\\"s here\""},
+
+ // strings must not contain quote characters, not even
backslash-escaped ones: HTML
+ // parsing ignores CSS escaping, so a stray quote of the other
kind breaks out of the
+ // style attribute the token is emitted into
+ {"'it\\'s here'", RUBBISH},
+ {"\"it\\\"s here\"", RUBBISH},
+ {"'\" onmouseover=\"alert(document.cookie) '", RUBBISH},
+ {"\"' onmouseover='alert(document.cookie) \"", RUBBISH},
+
+ // strings must not contain markup that could end a surrounding
<style> element
+ {"'</style><script>alert(1)</script>'", RUBBISH},
+ {"\"</style><script>alert(1)</script>\"", RUBBISH},
+
+ // the javascript: guard must tolerate arbitrary whitespace
+ {"'javascript :alert(1)'", RUBBISH},
+
+ // a backslash is excluded from the string content entirely,
independent of whether it is
+ // adjacent to a quote character - e.g. a CSS hex escape like \22
is no longer accepted
+ {"'\\22 '", RUBBISH},
+ {"'back\\slash'", RUBBISH},
+
+ // HTML entities inside a string are harmless and must still be
accepted: <style> raw text
+ // is never entity-decoded, so """ stays literal text and
cannot break out of a
+ // surrounding attribute the way a raw quote character would
+ {"'" onmouseover=alert(1)'", "'" onmouseover=alert(1)'"},
// invalid strings
{"\"bad string", RUBBISH},
@@ -669,6 +737,23 @@ public class XSSAPIImplTest {
// valid parenthesis
{"rgb(255, 255, 255)", "rgb(255, 255, 255)"},
+ {"translate(10px, 20px)", "translate(10px, 20px)"},
+ {"rgba(0,0,0,.5)", "rgba(0,0,0,.5)"},
+
+ // NUMBER/IDENTIFIER handoff boundaries that any rewrite must
still accept: an
+ // exponent marker with no following digits falls back to being
consumed as part of the
+ // identifier/unit instead of the (failed) exponent group, and a
trailing dot with no
+ // following digit is still a valid fractional NUMBER
+ {"2e2em", "2e2em"},
+ {"2epx", "2epx"},
+ {"5e-x", "5e-x"},
+ {"5.em", "5.em"},
+
+ // degenerate tokens that the old NUMBER (which could match the
empty string) used to
+ // accept as a bare sign or a lone dot; NUMBER must require at
least one digit now
+ {"+", RUBBISH},
+ {"-", RUBBISH},
+ {".", RUBBISH},
// invalid parenthesis
{"rgb(255, 255, 255", RUBBISH},
@@ -678,6 +763,11 @@ public class XSSAPIImplTest {
{"url(http://example.com/test.png)",
"url(http://example.com/test.png)"},
{"url('image/test.png')", "url('image/test.png')"},
+ // the URL production reuses STRING, so the same quote/markup
breakout protection must
+ // apply through url(...) as well
+ {"url('</style><script>alert(1)</script>')", RUBBISH},
+ {"url('\" onmouseover=\"alert(document.cookie) ')", RUBBISH},
+
// invalid tokens
{"color: red", RUBBISH}
};
@@ -713,7 +803,28 @@ public class XSSAPIImplTest {
// Source Expected Result
{null, RUBBISH},
{"blah */ hack", RUBBISH},
- {"Valid comment", "Valid comment"}
+ // sequences that end the surrounding <script>/<style> raw-text
element or shift the
+ // script-data tokenizer state must not survive validation
+ {"</script><script>alert(document.domain)//", RUBBISH},
+ {"blah </ScRiPt ><img src=x onerror=alert(1)>", RUBBISH},
+ {"</style><script>alert(1)</script>", RUBBISH},
+ {"blah </StYlE >", RUBBISH},
+ {"<!-- enters script-data-escaped state", RUBBISH},
+ {"leaves the escaped state -->", RUBBISH},
+ // a newline or tab before the tag-name-close is still a closing
tag to the HTML tokenizer
+ {"blah </script\nafter", RUBBISH},
+ {"blah </style\tafter", RUBBISH},
+ {"Valid comment", "Valid comment"},
+ {"Valid /* nested comment start", "Valid /* nested comment start"},
+ // sequences that must NOT be blocked: a backslash-escaped slash
is inert at the HTML
+ // tokenizer level (only a literal "</" ends the raw-text
element), a bare opening tag
+ // cannot end or nest inside the raw-text element it is already
inside of, and a
+ // near-miss that is one character short of an actual breakout
token is not a breakout
+ {"contains <\\/script escaped slash", "contains <\\/script escaped
slash"},
+ {"a bare <script> tag is not a closing tag", "a bare <script> tag
is not a closing tag"},
+ {"a bare <style> tag is not a closing tag", "a bare <style> tag is
not a closing tag"},
+ {"almost </scrip but not quite", "almost </scrip but not quite"},
+ {"almost <!- but not quite", "almost <!- but not quite"}
};
}
@@ -734,7 +845,28 @@ public class XSSAPIImplTest {
{"[]", "[]"},
{"[1,2]", "[1,2]"},
{"[1", RUBBISH_JSON},
- {"[{\"test\": \"test\"}]", "[{\"test\":\"test\"}]"}
+ {"[{\"test\": \"test\"}]", "[{\"test\":\"test\"}]"},
+ // values that could break out of an inline <script> element must
be escaped
+ // (semantics-preserving JSON escapes)
+ {
+ "{\"a\":\"</script><script>alert(1)</script>\"}",
+
"{\"a\":\"\\u003C/script>\\u003Cscript>alert(1)\\u003C/script>\"}"
+ },
+ {"[\"</script>\"]", "[\"\\u003C/script>\"]"},
+ {"{\"a\":\"x\u2028y\u2029z\"}", "{\"a\":\"x\\u2028y\\u2029z\"}"},
+
+ // a pre-escaped '<' in the input is decoded by the JSON parser to
a literal '<' and must
+ // be re-escaped on the way out, not passed through as-is
+ {"{\"a\":\"\\u003Cscript\\u003E\"}", "{\"a\":\"\\u003Cscript>\"}"},
+
+ // object keys go through the same serialized output and must be
escaped too, not just
+ // string values
+ {"{\"<script>x</script>\": \"v\"}",
"{\"\\u003Cscript>x\\u003C/script>\":\"v\"}"},
+
+ // '<!--' is neutralized once its '<' is escaped; a lone '-->'
with no preceding '<!--'
+ // contains no '<' and is left untouched
+ {"{\"a\":\"<!-- comment\"}", "{\"a\":\"\\u003C!-- comment\"}"},
+ {"[\"-->\",\"safe\"]", "[\"-->\",\"safe\"]"}
};
}
diff --git a/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
b/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
index 1a18b5d..01abd8d 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
@@ -71,6 +71,11 @@ public class XSSFilterImplTest {
});
testData.add(
new Object[]
{"javascript:alert(1)",
false});
+ // HTML5-only named character references (unknown to unescapeHtml4)
and numeric references
+ // without a terminating semicolon are still decoded by browsers
before URL parsing
+ testData.add(new Object[] {"java	script:alert(1)", false});
+ testData.add(new Object[] {"java
script:alert(1)",
false});
+ testData.add(new Object[] {"javascript:alert(1)", false});
testData.add(new Object[] {"%-12", false});
testData.add(new Object[] {"/promotion/25%/", false});
testData.add(new Object[] {"#", true});
@@ -218,6 +223,76 @@ public class XSSFilterImplTest {
"Expected the fallback sanitizer to remove javascript: hrefs,
but got: " + filtered);
}
+ @Test
+ public void
testUnicodeUnescaperDecodesHtml5NamedEntitiesNotOnlyTabNewlineAndColon() {
+ // spot-check a few of the HTML5-only named references beyond
	/
/: that are
+ // exercised by the javascript-scheme-bypass tests, to guard against
the entity table silently
+ // losing entries or mapping to the wrong character
+ assertEquals("a(b)c/d?e",
XSSFilterImpl.UNICODE_UNESCAPER.translate("a(b)c/d?e"));
+ assertEquals("[x]",
XSSFilterImpl.UNICODE_UNESCAPER.translate("[x]"));
+ }
+
+ @Test
+ public void testUnicodeUnescaperNamedEntitiesAreCaseSensitive() {
+ // HTML5 named references are case-sensitive; "&tab;" (lower-case) is
not a valid reference and
+ // must be left untouched, unlike "	"
+ assertEquals("&tab;",
XSSFilterImpl.UNICODE_UNESCAPER.translate("&tab;"));
+ assertEquals("\t", XSSFilterImpl.UNICODE_UNESCAPER.translate("	"));
+ }
+
+ @Test
+ public void testNumericEntityUnescaperDecimalReferenceWithoutSemicolon() {
+ assertEquals("javascript",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("javascript"));
+ }
+
+ @Test
+ public void testNumericEntityUnescaperDecimalReferenceWithSemicolon() {
+ assertEquals("javascript",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("javascript"));
+ }
+
+ @Test
+ public void testNumericEntityUnescaperHexReferenceWithoutSemicolon() {
+ assertEquals("Junk",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("Junk"));
+ }
+
+ @Test
+ public void testNumericEntityUnescaperHexReferenceWithSemicolon() {
+ assertEquals("Junk",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("Junk"));
+ }
+
+ @Test
+ public void
testNumericEntityUnescaperHexReferenceConsumesTrailingHexLetters() {
+ // unlike decimal references, a hex reference without a terminating
semicolon keeps consuming
+ // digits as long as they are valid hex digits - including letters
a-f/A-F - so "ڪbc" is
+ // parsed as the single hex value 0x6AA, not as 0x6A followed by the
literal text "a;bc"
+ String expected = new String(Character.toChars(0x6AA)) + "bc";
+ assertEquals(expected,
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("ڪbc"));
+ }
+
+ @Test
+ public void
testNumericEntityUnescaperOutOfRangeCodePointDecodesToReplacementCharacter() {
+ // browsers decode a numeric reference above the maximum Unicode code
point to U+FFFD rather
+ // than rejecting it
+ assertEquals("�",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("�"));
+ }
+
+ @Test
+ public void
testNumericEntityUnescaperOverflowingDigitsDecodeToReplacementCharacter() {
+ // a digit sequence too large to fit in an int must not throw
NumberFormatException out of the
+ // translator; it is treated the same as an out-of-range code point
+ assertEquals("�",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("�"));
+ }
+
+ @Test
+ public void
testNumericEntityUnescaperIncompleteHexReferenceIsLeftLiteral() {
+ assertEquals("&#x",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("&#x"));
+ }
+
+ @Test
+ public void
testNumericEntityUnescaperReferenceWithoutDigitsIsLeftLiteral() {
+ assertEquals("&#abc",
XSSFilterImpl.NUMERIC_ENTITY_UNESCAPER.translate("&#abc"));
+ }
+
private static @NotNull InputStream getPolicyFileAsStream() {
return Objects.requireNonNull(
XSSFilterImplTest.class.getClassLoader().getResourceAsStream(XSSFilterImpl.EMBEDDED_POLICY_PATH),
