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 1ccb986 SLING-13333 Fix potential HTML sanitization weakness in
fallback error-handling path
1ccb986 is described below
commit 1ccb98684672eb99d8db4245d83aeb939c6bb5b4
Author: Joerg Hoh <[email protected]>
AuthorDate: Mon Sep 14 17:06:09 2026 +0200
SLING-13333 Fix potential HTML sanitization weakness in fallback
error-handling path
---
.../org/apache/sling/xss/impl/HtmlSanitizer.java | 36 +++++++++++
.../sling/xss/impl/HtmlToHtmlContentContext.java | 14 ++++-
.../org/apache/sling/xss/impl/XSSFilterImpl.java | 23 +++++--
.../sling/xss/impl/xml/AntiSamyXmlParser.java | 17 ++++++
.../xss/impl/HtmlSanitizerMaxInputSizeTest.java | 70 +++++++++++++++++++++
.../xss/impl/HtmlToHtmlContentContextTest.java | 71 ++++++++++++++++++++++
.../apache/sling/xss/impl/XSSFilterImplTest.java | 42 ++++++++++++-
7 files changed, 264 insertions(+), 9 deletions(-)
diff --git a/src/main/java/org/apache/sling/xss/impl/HtmlSanitizer.java
b/src/main/java/org/apache/sling/xss/impl/HtmlSanitizer.java
index 6f49abb..68edc97 100644
--- a/src/main/java/org/apache/sling/xss/impl/HtmlSanitizer.java
+++ b/src/main/java/org/apache/sling/xss/impl/HtmlSanitizer.java
@@ -23,26 +23,62 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import org.apache.commons.lang3.StringUtils;
import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
import org.owasp.html.DynamicAttributesSanitizerPolicy;
import org.owasp.html.Handler;
import org.owasp.html.HtmlStreamEventReceiver;
import org.owasp.html.HtmlStreamRenderer;
import org.owasp.html.PolicyFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class HtmlSanitizer {
+ static final String MAX_INPUT_SIZE_DIRECTIVE = "maxInputSize";
+
+ private static final Logger LOG =
LoggerFactory.getLogger(HtmlSanitizer.class);
+
private AntiSamyPolicyAdapter customPolicy;
private Map policies;
private Set<String> textContainers;
+ private final int maxInputSize;
public HtmlSanitizer(AntiSamyPolicy policy) {
this.customPolicy = new AntiSamyPolicyAdapter(policy);
policies =
reflectionGetPolicies(customPolicy.getHtmlCleanerPolicyFactory());
textContainers =
reflectionGetTextContainers(customPolicy.getHtmlCleanerPolicyFactory());
+ maxInputSize = parseMaxInputSize(policy);
+ }
+
+ /**
+ * Reads the {@code maxInputSize} directive from the policy, matching the
AntiSamy contract that
+ * inputs larger than this limit are rejected. When the directive is
absent no limit is applied
+ * (backwards-compatible behavior); an unparseable value is reported and
ignored.
+ */
+ private static int parseMaxInputSize(AntiSamyPolicy policy) {
+ String value = policy.getDirectives().get(MAX_INPUT_SIZE_DIRECTIVE);
+ if (value != null) {
+ try {
+ return Integer.parseInt(value.trim());
+ } catch (NumberFormatException e) {
+ LOG.warn("Ignoring invalid value '{}' of the {} policy
directive.", value, MAX_INPUT_SIZE_DIRECTIVE);
+ }
+ }
+ return Integer.MAX_VALUE;
}
public SanitizedResult scan(String taintedHTML) {
+ if (taintedHTML.length() > maxInputSize) {
+ // fail closed, matching the AntiSamy semantics of the
maxInputSize directive: an empty
+ // result with an error makes filter() return an empty string and
check() return false
+ LOG.warn(
+ "Rejecting input of {} characters as it exceeds the {}
policy directive value of {}.",
+ taintedHTML.length(),
+ MAX_INPUT_SIZE_DIRECTIVE,
+ maxInputSize);
+ return new SanitizedResult(StringUtils.EMPTY, 1);
+ }
StringBuilder sb = new StringBuilder(taintedHTML.length());
HtmlStreamEventReceiver out = HtmlStreamRenderer.create(sb,
Handler.DO_NOTHING);
DynamicAttributesSanitizerPolicy dynamicPolicy = new
DynamicAttributesSanitizerPolicy(
diff --git
a/src/main/java/org/apache/sling/xss/impl/HtmlToHtmlContentContext.java
b/src/main/java/org/apache/sling/xss/impl/HtmlToHtmlContentContext.java
index 54a0a5d..d6cdabd 100644
--- a/src/main/java/org/apache/sling/xss/impl/HtmlToHtmlContentContext.java
+++ b/src/main/java/org/apache/sling/xss/impl/HtmlToHtmlContentContext.java
@@ -87,8 +87,18 @@ public class HtmlToHtmlContentContext implements
XSSFilterRule {
log.debug(
"Will perform a second attempt at filtering the following
input due to a StackOverflowError:\n{}",
input);
- results = handler.getFallbackHtmlSanitizer().scan(input);
- log.debug("Second attempt was successful.");
+ try {
+ results = handler.getFallbackHtmlSanitizer().scan(input);
+ log.debug("Second attempt was successful.");
+ } catch (StackOverflowError inner) {
+ // fail closed instead of letting the Error escape into
application code; a
+ // SanitizedResult with an error makes filter() return an
empty string and
+ // check() return false
+ log.warn(
+ "Second filtering attempt failed with a
StackOverflowError as well; rejecting the input (fail-closed).");
+ log.debug("Provided input: {}", input);
+ results = new SanitizedResult(StringUtils.EMPTY, 1);
+ }
}
return results;
}
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 a3d5d9c..7bafd38 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
@@ -141,10 +141,25 @@ public class XSSFilterImpl implements XSSFilter {
"(?!\\s*javascript(?::|:))" + RELATIVE_PART + "?(?:\\?" +
QUERY + ")?(?:#" + FRAGMENT + ")?";
public static final String URI = SCHEME_PATTERN + ":" + HIER_PART +
"(?:\\?" + QUERY + ")?(?:#" + FRAGMENT + ")?";
- static final Pattern ON_SITE_SIMPLIFIED =
-
Pattern.compile("([\\p{L}\\p{N}\\\\\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*|\\#"
+ "(\\w)+)");
- static final Pattern OFF_SITE_SIMPLIFIED =
Pattern.compile("(\\s)*((ht|f)tp(s?)://|mailto:)"
- +
"[\\p{L}\\p{N}]+[\\p{L}\\p{N}\\p{Zs}\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*(\\s)*");
+ /*
+ * The simplified patterns are only used when the primary RFC 3986-shaped
regexes abort with a
+ * StackOverflowError on pathological input (see runHrefValidation and
FallbackATag). A degraded
+ * fallback must never be more permissive than the primary path for scheme
safety, so the same
+ * javascript-scheme guard used by RELATIVE_REF is applied here as well.
The guard is
+ * case-insensitive as defense in depth for consumers that do not
lower-case the value first.
+ */
+ static final Pattern ON_SITE_SIMPLIFIED =
Pattern.compile("(?!\\s*(?i:javascript)(?::|:))"
+ +
"([\\p{L}\\p{N}\\\\\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*|\\#" +
"(\\w)+)");
+ /*
+ * The quantifiers below are possessive (`*+` / `++`) on purpose: the
three quantified parts overlap
+ * ([\p{L}\p{N}] is a subset of the following character class, which in
turn overlaps the trailing
+ * (\s)* on space characters), so with regular greedy quantifiers a
non-matching input such as
+ * "http://"; + "a".repeat(n) + "^" triggers polynomial backtracking
(O(n^2) and worse). Because each
+ * quantifier iteration consumes exactly one character from a character
class, making them possessive
+ * does not change the accepted language - it only removes the
backtracking, keeping matching linear.
+ */
+ static final Pattern OFF_SITE_SIMPLIFIED =
Pattern.compile("(\\s)*+((ht|f)tp(s?)://|mailto:)"
+ +
"[\\p{L}\\p{N}]++[\\p{L}\\p{N}\\p{Zs}\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*+(\\s)*+");
static final Attribute FALLBACK_HREF_ATTRIBUTE = new Attribute(
"href",
diff --git a/src/main/java/org/apache/sling/xss/impl/xml/AntiSamyXmlParser.java
b/src/main/java/org/apache/sling/xss/impl/xml/AntiSamyXmlParser.java
index c91eef3..74d1cd4 100644
--- a/src/main/java/org/apache/sling/xss/impl/xml/AntiSamyXmlParser.java
+++ b/src/main/java/org/apache/sling/xss/impl/xml/AntiSamyXmlParser.java
@@ -25,6 +25,9 @@ import javax.xml.stream.XMLStreamReader;
import java.io.IOException;
import java.io.InputStream;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import org.slf4j.Logger;
@@ -34,6 +37,12 @@ public class AntiSamyXmlParser {
private static final String DIRECTIVE_EMBED_STYLE_SHEETS =
"embedStyleSheets";
+ /**
+ * The directives this implementation actually enforces; {@code
embedStyleSheets} is handled
+ * separately (only its safe {@code false} value is supported).
+ */
+ private static final List<String> ENFORCED_DIRECTIVES =
Arrays.asList("allowDynamicAttributes", "maxInputSize");
+
private final Logger logger = LoggerFactory.getLogger(getClass());
public AntiSamyRules createRules(InputStream input) throws
XMLStreamException, IOException {
@@ -50,6 +59,14 @@ public class AntiSamyXmlParser {
"Unsupported configuration directive {} is set to true and
will be ignored",
DIRECTIVE_EMBED_STYLE_SHEETS);
}
+ List<String> ignoredDirectives =
rules.getDirectivesByName().keySet().stream()
+ .filter(name -> !ENFORCED_DIRECTIVES.contains(name) &&
!DIRECTIVE_EMBED_STYLE_SHEETS.equals(name))
+ .collect(Collectors.toList());
+ if (!ignoredDirectives.isEmpty()) {
+ logger.warn(
+ "The configuration directives {} are not enforced by this
implementation and will be ignored",
+ ignoredDirectives);
+ }
xmlStreamReader.close();
return rules;
}
diff --git
a/src/test/java/org/apache/sling/xss/impl/HtmlSanitizerMaxInputSizeTest.java
b/src/test/java/org/apache/sling/xss/impl/HtmlSanitizerMaxInputSizeTest.java
new file mode 100644
index 0000000..3fe6e7a
--- /dev/null
+++ b/src/test/java/org/apache/sling/xss/impl/HtmlSanitizerMaxInputSizeTest.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.xss.impl;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Verifies that the {@code maxInputSize} directive from the policy file is
actually enforced: the
+ * embedded policy declares a limit of 200000 characters and inputs above it
must be rejected
+ * (fail-closed) instead of being processed.
+ */
+public class HtmlSanitizerMaxInputSizeTest {
+
+ public static final String POLICY_FILE = "SLING-INF/content/config.xml";
+
+ // must match the maxInputSize directive of the embedded policy file
+ private static final int EMBEDDED_POLICY_MAX_INPUT_SIZE = 200000;
+
+ private static HtmlSanitizer antiSamy;
+
+ @BeforeAll
+ public static void setup() throws InvalidConfigException,
XMLStreamException, IOException {
+ antiSamy = new HtmlSanitizer(new AntiSamyPolicy(
+
HtmlSanitizerMaxInputSizeTest.class.getClassLoader().getResourceAsStream(POLICY_FILE)));
+ }
+
+ @Test
+ public void testInputBelowMaxInputSizeIsProcessed() {
+ String input = "<p>" + StringUtils.repeat('a', 1000) + "</p>";
+ SanitizedResult result = antiSamy.scan(input);
+ assertEquals(0, result.getNumberOfErrors());
+
assertTrue(result.getSanitizedString().contains(StringUtils.repeat('a', 1000)));
+ }
+
+ @Test
+ public void testOversizedInputIsRejected() {
+ String input = "<p>" + StringUtils.repeat('a',
EMBEDDED_POLICY_MAX_INPUT_SIZE) + "</p>";
+ SanitizedResult result = antiSamy.scan(input);
+ assertTrue(result.getNumberOfErrors() > 0, "Expected an error for an
input exceeding maxInputSize.");
+ assertTrue(
+ StringUtils.isEmpty(result.getSanitizedString()),
+ "Expected empty filtered output for an input exceeding
maxInputSize.");
+ }
+}
diff --git
a/src/test/java/org/apache/sling/xss/impl/HtmlToHtmlContentContextTest.java
b/src/test/java/org/apache/sling/xss/impl/HtmlToHtmlContentContextTest.java
new file mode 100644
index 0000000..95bfb8f
--- /dev/null
+++ b/src/test/java/org/apache/sling/xss/impl/HtmlToHtmlContentContextTest.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.xss.impl;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class HtmlToHtmlContentContextTest {
+
+ private static final String INPUT = "<p>some input</p>";
+
+ private PolicyHandler policyHandlerThrowing(boolean fallbackThrows) {
+ HtmlSanitizer throwingSanitizer = mock(HtmlSanitizer.class);
+ when(throwingSanitizer.scan(anyString())).thenThrow(new
StackOverflowError());
+
+ HtmlSanitizer fallbackSanitizer;
+ if (fallbackThrows) {
+ fallbackSanitizer = throwingSanitizer;
+ } else {
+ fallbackSanitizer = mock(HtmlSanitizer.class);
+ when(fallbackSanitizer.scan(anyString())).thenReturn(new
SanitizedResult(INPUT, 0));
+ }
+
+ PolicyHandler policyHandler = mock(PolicyHandler.class);
+ when(policyHandler.getHtmlSanitizer()).thenReturn(throwingSanitizer);
+
when(policyHandler.getFallbackHtmlSanitizer()).thenReturn(fallbackSanitizer);
+ return policyHandler;
+ }
+
+ @Test
+ public void testFallbackIsUsedOnStackOverflowError() {
+ PolicyHandler policyHandler = policyHandlerThrowing(false);
+ HtmlToHtmlContentContext context = new HtmlToHtmlContentContext();
+ assertEquals(INPUT, context.filter(policyHandler, INPUT));
+ assertTrue(context.check(policyHandler, INPUT));
+ }
+
+ /**
+ * SLING - a StackOverflowError thrown by the fallback sanitizer as well
must not escape into
+ * application code: filter() has to fail closed with an empty string and
check() with false.
+ */
+ @Test
+ public void testSecondStackOverflowErrorFailsClosed() {
+ PolicyHandler policyHandler = policyHandlerThrowing(true);
+ HtmlToHtmlContentContext context = new HtmlToHtmlContentContext();
+ assertEquals("", context.filter(policyHandler, INPUT));
+ assertFalse(context.check(policyHandler, INPUT));
+ }
+}
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 12c7513..1a18b5d 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSFilterImplTest.java
@@ -95,6 +95,9 @@ public class XSSFilterImplTest {
return testData;
}
+ private static final String FALLBACK_TRIGGERING_CONTENT =
+ "<a href=\"https://sling.apache.org"; + "/a".repeat(1300) +
".\">Click</a>";
+
public SlingContext context = new
SlingContext(ResourceResolverType.JCR_MOCK);
private XSSFilterImpl xssFilter;
@@ -175,11 +178,44 @@ public class XSSFilterImplTest {
}
}
+ @Test
+ public void testFallbackHrefRegexesDoNotBacktrackPolynomially() {
+ // quadratic variant: scheme prefix, long run of characters shared by
the overlapping
+ // quantified character classes, then a character outside all of them
+ String quadratic = "http://"; + "a".repeat(100000) + "^";
+ // cubic variant: mailto scheme, letters, then a long run of spaces
(matched by both the
+ // middle character class and the trailing whitespace quantifier),
then a non-matching char
+ String cubic = "mailto:"; + "a".repeat(20000) + " ".repeat(40000) + "^";
+ for (String input : new String[] {quadratic, cubic}) {
+ long start = System.nanoTime();
+
assertFalse(XSSFilterImpl.OFF_SITE_SIMPLIFIED.matcher(input).matches());
+ long elapsedMillis = (System.nanoTime() - start) / 1_000_000L;
+ // linear matching finishes in a few milliseconds; polynomial
backtracking needs
+ // minutes to hours for inputs of this size
+ assertTrue(
+ elapsedMillis < 5000,
+ "Expected linear-time rejection of a " + input.length() +
" character URL, but matching took "
+ + elapsedMillis + "ms.");
+ }
+ }
+
@Test
public void testFallbackFiltering() {
- final String longURLContext = "<a href=\"https://sling.apache.org";
- +
"/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
[...]
- assertEquals(longURLContext, xssFilter.filter(longURLContext));
+ assertEquals(FALLBACK_TRIGGERING_CONTENT,
xssFilter.filter(FALLBACK_TRIGGERING_CONTENT));
+ }
+
+ @Test
+ public void testFallbackFilteringDoesNotAllowJavascriptHrefs() {
+ // the first anchor makes the primary sanitizer scan abort with a
StackOverflowError, so the
+ // whole input (including the second and third anchors) is re-scanned
with the fallback
+ // policy; the simplified fallback href patterns must not keep a
javascript: URL alive
+ final String input = FALLBACK_TRIGGERING_CONTENT
+ + "<a href=\"javascript:alert(document.domain)\">plain</a>"
+ + "<a href=\"JaVaScRiPt:alert(document.domain)\">mixed
case</a>";
+ final String filtered = xssFilter.filter(input);
+ assertFalse(
+
filtered.toLowerCase(java.util.Locale.ROOT).contains("javascript"),
+ "Expected the fallback sanitizer to remove javascript: hrefs,
but got: " + filtered);
}
private static @NotNull InputStream getPolicyFileAsStream() {
