Author: radu
Date: Mon Jun 15 08:04:53 2015
New Revision: 1685504
URL: http://svn.apache.org/r1685504
Log:
SLING-4584 - Performance: XSSAPI.getValidHref should not be based on HTML
filtering
* Instead of parsing a constructed temporary anchor tag, skip the HTML parsing
and validate the URL directly for better performance.
(patch contributed by Joel Richard <joelr...@adobe.com>)
Added:
sling/trunk/bundles/extensions/xss/src/test/resources/
sling/trunk/bundles/extensions/xss/src/test/resources/configWithoutHref.xml
Modified:
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/XSSFilter.java
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/package-info.java
sling/trunk/bundles/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
Modified:
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/XSSFilter.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/XSSFilter.java?rev=1685504&r1=1685503&r2=1685504&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/XSSFilter.java
(original)
+++
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/XSSFilter.java
Mon Jun 15 08:04:53 2015
@@ -59,4 +59,15 @@ public interface XSSFilter {
* @throws NullPointerException if context is <code>null</code>
*/
String filter(ProtectionContext context, String src);
+
+ /**
+ * Checks if the given URL is valid to be used for the <code>href</code>
attribute in a <code>a</code> tag.
+ * <p/>
+ * The default protection context is used for checking.
+ *
+ * @param url the URL that should be validated
+ * @return true if the URL is violation-free
+ */
+ boolean isValidHref(String url);
+
}
Modified:
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java?rev=1685504&r1=1685503&r2=1685504&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
(original)
+++
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
Mon Jun 15 08:04:53 2015
@@ -207,14 +207,10 @@ public class XSSAPIImpl implements XSSAP
if (qMarkIx > 0) {
encodedUrl = encodedUrl.substring(0, qMarkIx) +
encodedUrl.substring(qMarkIx).replaceAll(":", "%3A");
}
- String testHtml = LINK_PREFIX + mangleNamespaces(encodedUrl) +
LINK_SUFFIX;
- // replace all & with & because filterHTML will also apply
this encoding
- testHtml = testHtml.replaceAll("&(?!amp)", "&");
- final String safeHtml =
xssFilter.filter(ProtectionContext.HTML_HTML_CONTENT, testHtml);
- // if the xssFilter didn't like the input string we just return ""
- // otherwise we return the mangled url without encoding
- if (safeHtml.equals(testHtml)) {
- return mangleNamespaces(encodedUrl);
+
+ encodedUrl = mangleNamespaces(encodedUrl);
+ if (xssFilter.isValidHref(encodedUrl)) {
+ return encodedUrl;
}
}
Modified:
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java?rev=1685504&r1=1685503&r2=1685504&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
(original)
+++
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
Mon Jun 15 08:04:53 2015
@@ -17,8 +17,11 @@
package org.apache.sling.xss.impl;
import java.io.InputStream;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Pattern;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
@@ -35,6 +38,8 @@ import org.apache.sling.xss.XSSFilter;
import org.osgi.service.event.Event;
import org.osgi.service.event.EventConstants;
import org.osgi.service.event.EventHandler;
+import org.owasp.validator.html.model.Attribute;
+import org.owasp.validator.html.model.Tag;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -49,9 +54,21 @@ public class XSSFilterImpl implements XS
private static final Logger LOGGER =
LoggerFactory.getLogger(XSSFilterImpl.class);
+ // 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{N}\\\\\\.\\#@\\$%\\+&;\\-_~,\\?=/!\\*\\(\\)]*|\\#(\\w)+)"),
+
Pattern.compile("(\\s)*((ht|f)tp(s?)://|mailto:)[\\p{L}\\p{N}]+[\\p{L}\\p{N}\\p{Zs}\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\*\\(\\)]*(\\s)*")
+ ),
+ Collections.<String>emptyList(),
+ "removeAttribute", ""
+ );
+
private static final String DEFAULT_POLICY_PATH = "sling/xss/config.xml";
private static final int DEFAULT_POLICY_CACHE_SIZE = 128;
private PolicyHandler defaultHandler;
+ private Attribute hrefAttribute;
// available contexts
private final XSSFilterRule htmlHtmlContext = new
HtmlToHtmlContentContext();
@@ -104,7 +121,7 @@ public class XSSFilterImpl implements XS
if (policyStream != null) {
try {
if (defaultHandler == null) {
- defaultHandler = new PolicyHandler(policyStream);
+ setDefaultHandler(new PolicyHandler(policyStream));
policyStream.close();
}
} catch (Exception e) {
@@ -119,7 +136,7 @@ public class XSSFilterImpl implements XS
if (policyStream != null) {
try {
if (defaultHandler == null) {
- defaultHandler = new PolicyHandler(policyStream);
+ setDefaultHandler(new PolicyHandler(policyStream));
policyStream.close();
}
} catch (Exception e) {
@@ -185,7 +202,19 @@ public class XSSFilterImpl implements XS
@SuppressWarnings("unused")
public void setDefaultPolicy(InputStream policyStream) throws Exception {
- defaultHandler = new PolicyHandler(policyStream);
+ setDefaultHandler(new PolicyHandler(policyStream));
+ }
+
+ private void setDefaultHandler(PolicyHandler defaultHandler) {
+ Tag linkTag = defaultHandler.getPolicy().getTagByLowercaseName("a");
+ Attribute hrefAttribute = (linkTag != null) ?
linkTag.getAttributeByName("href") : null;
+ if (hrefAttribute == null) {
+ // Fallback to default configuration
+ hrefAttribute = DEFAULT_HREF_ATTRIBUTE;
+ }
+
+ this.defaultHandler = defaultHandler;
+ this.hrefAttribute = hrefAttribute;
}
@SuppressWarnings("unused")
@@ -210,4 +239,15 @@ public class XSSFilterImpl implements XS
public boolean hasPolicy(String policyName) {
return policies.containsKey(policyName);
}
+
+ @Override
+ public boolean isValidHref(String url) {
+ // Same logic as in
org.owasp.validator.html.scan.MagicSAXFilter.startElement()
+ boolean isValid =
hrefAttribute.containsAllowedValue(url.toLowerCase());
+ if (!isValid) {
+ isValid = hrefAttribute.matchesAllowedExpression(url);
+ }
+ return isValid;
+ }
+
}
Modified:
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/package-info.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/package-info.java?rev=1685504&r1=1685503&r2=1685504&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/package-info.java
(original)
+++
sling/trunk/bundles/extensions/xss/src/main/java/org/apache/sling/xss/package-info.java
Mon Jun 15 08:04:53 2015
@@ -17,9 +17,9 @@
/**
* XSS Protection Service
*
- * @version 1.1.0
+ * @version 1.2.0
*/
-@Version("1.1.0")
+@Version("1.2.0")
package org.apache.sling.xss;
import aQute.bnd.annotation.Version;
Modified:
sling/trunk/bundles/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java?rev=1685504&r1=1685503&r2=1685504&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
(original)
+++
sling/trunk/bundles/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
Mon Jun 15 08:04:53 2015
@@ -23,12 +23,14 @@ import java.lang.reflect.Field;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.xss.XSSAPI;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.owasp.validator.html.AntiSamy;
import org.owasp.validator.html.Policy;
+import org.owasp.validator.html.model.Attribute;
import org.powermock.reflect.Whitebox;
import junit.framework.TestCase;
@@ -49,16 +51,8 @@ public class XSSAPIImplTest {
@Before
public void setup() {
try {
- InputStream policyStream = new
FileInputStream("./src/main/resources/SLING-INF/content/config.xml");
- Policy policy = Policy.getInstance(policyStream);
- AntiSamy antiSamy = new AntiSamy(policy);
-
- PolicyHandler mockPolicyHandler = mock(PolicyHandler.class);
- when(mockPolicyHandler.getPolicy()).thenReturn(policy);
- when(mockPolicyHandler.getAntiSamy()).thenReturn(antiSamy);
-
XSSFilterImpl xssFilter = new XSSFilterImpl();
- Whitebox.setInternalState(xssFilter, "defaultHandler",
mockPolicyHandler);
+ setDefaultHandler(xssFilter,
"./src/main/resources/SLING-INF/content/config.xml");
xssAPI = new XSSAPIImpl();
Whitebox.invokeMethod(xssAPI, "activate");
@@ -84,6 +78,18 @@ public class XSSAPIImplTest {
}
}
+ private static void setDefaultHandler(XSSFilterImpl xssFilter, String
filename) throws Exception {
+ InputStream policyStream = new FileInputStream(filename);
+ Policy policy = Policy.getInstance(policyStream);
+ AntiSamy antiSamy = new AntiSamy(policy);
+
+ PolicyHandler mockPolicyHandler = mock(PolicyHandler.class);
+ when(mockPolicyHandler.getPolicy()).thenReturn(policy);
+ when(mockPolicyHandler.getAntiSamy()).thenReturn(antiSamy);
+
+ Whitebox.invokeMethod(xssFilter, "setDefaultHandler",
mockPolicyHandler);
+ }
+
@Test
public void testEncodeForHTML() {
String[][] testData = {
@@ -276,6 +282,19 @@ public class XSSAPIImplTest {
}
@Test
+ public void testGetValidHrefWithoutHrefConfig() throws Exception {
+ // Load AntiSamy configuration without href filter
+ XSSFilterImpl xssFilter = Whitebox.getInternalState(xssAPI,
"xssFilter");
+ setDefaultHandler(xssFilter,
"./src/test/resources/configWithoutHref.xml");
+
+ Attribute hrefAttribute = Whitebox.getInternalState(xssFilter,
"hrefAttribute");
+ Assert.assertEquals(hrefAttribute,
XSSFilterImpl.DEFAULT_HREF_ATTRIBUTE);
+
+ // Run same tests again to check default configuration
+ testGetValidHref();
+ }
+
+ @Test
public void testGetValidInteger() {
String[][] testData = {
// Source
Expected Result
