CVE-2016-5394

@@ -23,7 +23,7 @@
     <parent>
         <groupId>org.apache.sling</groupId>
         <artifactId>sling</artifactId>
-        <version>26</version>
+        <version>28</version>
         <relativePath />
     </parent>
 
@@ -236,17 +236,13 @@
 
         <dependency>
             <groupId>javax.servlet</groupId>
-            <artifactId>servlet-api</artifactId>
+            <artifactId>javax.servlet-api</artifactId>
             <scope>provided</scope>
         </dependency>
 
         <dependency>
             <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.core</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.compendium</artifactId>
+            <artifactId>osgi.core</artifactId>
         </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
@@ -255,7 +251,7 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.api</artifactId>
-            <version>2.2.0</version>
+            <version>2.11.0</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
@@ -264,6 +260,12 @@
             <version>2.0.6</version>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+          <groupId>org.apache.sling</groupId>
+          <artifactId>org.apache.sling.serviceusermapper</artifactId>
+          <version>1.2.0</version>
+          <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>com.google.code.findbugs</groupId>
             <artifactId>jsr305</artifactId>
@@ -277,14 +279,13 @@
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-all</artifactId>
-            <version>1.8.4</version>
-            <type>jar</type>
+            <version>1.10.19</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.powermock</groupId>
             <artifactId>powermock-api-mockito</artifactId>
-            <version>1.5.5</version>
+            <version>1.6.5</version>
             <scope>test</scope>
         </dependency>
         <dependency>

@@ -21,6 +21,7 @@
 import java.util.regex.Pattern;
 
 import javax.annotation.Nonnull;
+import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 
@@ -43,6 +44,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.InputSource;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
 import org.xml.sax.XMLReader;
 
 @Component
@@ -65,6 +68,13 @@ protected void activate() {
         factory = SAXParserFactory.newInstance();
         factory.setValidating(false);
         factory.setNamespaceAware(true);
+        try {
+            factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        } catch (Exception e) {
+            LOGGER.error("SAX parser configuration error: " + e.getMessage(), e);
+        }
     }
 
     @Deactivate
@@ -113,6 +123,23 @@ public Long getValidLong(String source, long defaultValue) {
         return defaultValue;
     }
 
+    /**
+     * @see org.apache.sling.xss.XSSAPI#getValidDouble(String, double)
+     */
+    @Override
+    public Double getValidDouble(String source, double defaultValue) {
+        if (source != null && source.length() > 0) {
+            try {
+                return validator.getValidDouble("XSS", source, 0d, Double.MAX_VALUE, false);
+            } catch (Exception e) {
+                // ignore
+            }
+        }
+
+        // fall through to default if empty, null, or validation failure
+        return defaultValue;
+    }
+
     /**
      * @see org.apache.sling.xss.XSSAPI#getValidDimension(String, String)
      */
@@ -412,7 +439,7 @@ public String encodeForXMLAttr(String source) {
      */
     @Override
     public String encodeForJSString(String source) {
-        return source == null ? null : Encode.forJavaScriptSource(source);
+        return source == null ? null : Encode.forJavaScript(source).replace("\\-", "\\u002D");
     }
 
     /**

@@ -19,25 +19,28 @@
 import java.io.InputStream;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
 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;
+import org.apache.felix.scr.annotations.Properties;
 import org.apache.felix.scr.annotations.Property;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.Service;
-import org.apache.sling.api.SlingConstants;
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.api.resource.observation.ExternalResourceChangeListener;
+import org.apache.sling.api.resource.observation.ResourceChange;
+import org.apache.sling.api.resource.observation.ResourceChangeListener;
+import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.apache.sling.xss.ProtectionContext;
 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;
@@ -48,24 +51,28 @@
  * <a href="http://code.google.com/p/owaspantisamy/">http://code.google.com/p/owaspantisamy/</a>.
  */
 @Component(immediate = true)
-@Service(value = {EventHandler.class, XSSFilter.class})
-@Property(name = EventConstants.EVENT_TOPIC, value = {"org/apache/sling/api/resource/Resource/*"})
-public class XSSFilterImpl implements XSSFilter, EventHandler {
+@Service(value = {ResourceChangeListener.class, XSSFilter.class})
+@Properties({
+    @Property(name = ResourceChangeListener.CHANGES, value = {"ADDED", "CHANGED", "REMOVED"}),
+    @Property(name = ResourceChangeListener.PATHS, value = XSSFilterImpl.DEFAULT_POLICY_PATH)
+})
+public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, ExternalResourceChangeListener {
 
     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)*")
+                    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)*")
             ),
             Collections.<String>emptyList(),
             "removeAttribute", ""
     );
 
-    private static final String DEFAULT_POLICY_PATH = "sling/xss/config.xml";
+    public static final String DEFAULT_POLICY_PATH = "sling/xss/config.xml";
+    private static final String EMBEDDED_POLICY_PATH = "SLING-INF/content/config.xml";
     private static final int DEFAULT_POLICY_CACHE_SIZE = 128;
     private PolicyHandler defaultHandler;
     private Attribute hrefAttribute;
@@ -80,12 +87,16 @@ public class XSSFilterImpl implements XSSFilter, EventHandler {
     @Reference
     private ResourceResolverFactory resourceResolverFactory = null;
 
+    @Reference
+    private ServiceUserMapped serviceUserMapped;
+
     @Override
-    public void handleEvent(final Event event) {
-        final String path = (String) event.getProperty(SlingConstants.PROPERTY_PATH);
-        if (path.endsWith("/" + DEFAULT_POLICY_PATH)) {
-            LOGGER.debug("Detected policy file change at {}. Updating default handler.", path);
-            updateDefaultHandler();
+    public void onChange(List<ResourceChange> resourceChanges) {
+        for (ResourceChange change : resourceChanges) {
+            if (change.getPath().endsWith(DEFAULT_POLICY_PATH)) {
+                LOGGER.info("Detected policy file change ({}) at {}. Updating default handler.", change.getType().name(), change.getPath());
+                updateDefaultHandler();
+            }
         }
     }
 
@@ -111,37 +122,40 @@ protected void activate() {
         updateDefaultHandler();
     }
 
-    private void updateDefaultHandler() {
-        ResourceResolver adminResolver = null;
+    private synchronized void updateDefaultHandler() {
+        this.defaultHandler = null;
+        ResourceResolver xssResourceResolver = null;
         try {
-            adminResolver = resourceResolverFactory.getAdministrativeResourceResolver(null);
-            Resource policyResource = adminResolver.getResource(DEFAULT_POLICY_PATH);
+            xssResourceResolver = resourceResolverFactory.getServiceResourceResolver(null);
+            Resource policyResource = xssResourceResolver.getResource(DEFAULT_POLICY_PATH);
             if (policyResource != null) {
-                InputStream policyStream = policyResource.adaptTo(InputStream.class);
-                if (policyStream != null) {
-                    try {
-                        if (defaultHandler == null) {
-                            setDefaultHandler(new PolicyHandler(policyStream));
-                            policyStream.close();
+                try (InputStream policyStream = policyResource.adaptTo(InputStream.class)) {
+                    setDefaultHandler(new PolicyHandler(policyStream));
+                    LOGGER.info("Installed default policy from {}.", policyResource.getPath());
+                } catch (Exception e) {
+                    Throwable[] suppressed = e.getSuppressed();
+                    if (suppressed.length > 0) {
+                        for (Throwable t : suppressed) {
+                            LOGGER.error("Unable to load policy from " + policyResource.getPath(), t);
                         }
-                    } catch (Exception e) {
-                        LOGGER.error("Unable to load policy from " + policyResource.getPath(), e);
                     }
+                    LOGGER.error("Unable to load policy from " + policyResource.getPath(), e);
                 }
             } else {
                 // the content was not installed but the service is active; let's use the embedded file for the default handler
-                LOGGER.debug("Could not find a policy file at the default location {}. Attempting to use the default resource embedded in" +
+                LOGGER.warn("Could not find a policy file at the default location {}. Attempting to use the default resource embedded in" +
                         " the bundle.", DEFAULT_POLICY_PATH);
-                InputStream policyStream = this.getClass().getClassLoader().getResourceAsStream("SLING-INF/content/config.xml");
-                if (policyStream != null) {
-                    try {
-                        if (defaultHandler == null) {
-                            setDefaultHandler(new PolicyHandler(policyStream));
-                            policyStream.close();
+                try (InputStream policyStream = this.getClass().getClassLoader().getResourceAsStream(EMBEDDED_POLICY_PATH)) {
+                    setDefaultHandler(new PolicyHandler(policyStream));
+                    LOGGER.info("Installed default policy from the embedded {} file from the bundle.", EMBEDDED_POLICY_PATH);
+                } catch (Exception e) {
+                    Throwable[] suppressed = e.getSuppressed();
+                    if (suppressed.length > 0) {
+                        for (Throwable t : suppressed) {
+                            LOGGER.error("Unable to load policy from embedded policy file.", t);
                         }
-                    } catch (Exception e) {
-                        LOGGER.error("Unable to load policy from embedded policy file.", e);
                     }
+                    LOGGER.error("Unable to load policy from embedded policy file.", e);
                 }
             }
             if (defaultHandler == null) {
@@ -150,8 +164,8 @@ private void updateDefaultHandler() {
         } catch (LoginException e) {
             LOGGER.error("Unable to load the default policy file.", e);
         } finally {
-            if (adminResolver != null) {
-                adminResolver.close();
+            if (xssResourceResolver != null) {
+                xssResourceResolver.close();
             }
         }
     }
@@ -249,5 +263,4 @@ public boolean isValidHref(String url) {
         }
         return isValid;
     }
-
 }

@@ -17,9 +17,9 @@
 /**
  * XSS Protection Service
  *
- * @version 1.1.0
+ * @version 1.2.0
  */
-@Version("1.1.1")
+@Version("1.2.0")
 package org.apache.sling.xss;
 
 import aQute.bnd.annotation.Version;

@@ -67,6 +67,17 @@ public interface XSSAPI {
     @Nullable
     Long getValidLong(@Nullable String source,long defaultValue);
 
+    /**
+     * Validate a string which should contain an double, returning a default value if the source is
+     * {@code null}, empty, can't be parsed, or contains XSS risks.
+     *
+     * @param source      the source double
+     * @param defaultValue a default value if the source can't be used, is {@code null} or an empty string
+     * @return a sanitized double
+     */
+    @Nullable
+    Double getValidDouble(@Nullable String source, double defaultValue);
+
     /**
      * Validate a string which should contain a dimension, returning a default value if the source is
      * empty, can't be parsed, or contains XSS risks.  Allows integer dimensions and the keyword "auto".

@@ -67,9 +67,8 @@ 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{N}\\\.\#@\$%\+&amp;;\-_~,\?=/!\*\(\)]*|\#(\w)+)"/>
-        <regexp name="offsiteURL"
-                value="(\s)*((ht|f)tp(s?)://|mailto:)[\p{L}\p{N}]+[\p{L}\p{N}\p{Zs}\.\#@\$%\+&amp;;:\-_~,\?=/!\*\(\)]*(\s)*"/>
+        <regexp name="onsiteURL" value="([\p{L}\p{M}*+\p{N}\\\.\#@\$%\+&amp;;\-_~,\?=/!\*\(\)]*|\#(\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}\.\#@\$%\+&amp;;:\-_~,\?=/!\*\(\)]*(\s)*"/>
 
         <regexp name="boolean" value="(true|false)"/>
         <regexp name="singlePrintable" value="[a-zA-Z0-9]{1}"/>

@@ -204,7 +204,9 @@ public void testFilterHTML() {
                 {"<strike>strike</strike>", "<strike>strike</strike>"},
                 {"<s>s</s>", "<s>s</s>"},
 
-                {"<a href=\"\">empty href</a>", "<a href=\"\">empty href</a>"}
+                {"<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>"},
         };
 
         for (String[] aTestData : testData) {
@@ -220,6 +222,8 @@ public void testGetValidHref() {
         String[][] testData = {
                 //         Href                                        Expected Result
                 //
+                {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
+                {"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995", "/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
                 {null, ""},
                 {"", ""},
                 {"simple", "simple"},
@@ -340,6 +344,28 @@ public void testGetValidLong() {
         }
     }
 
+    @Test
+    public void testGetValidDouble() {
+        String[][] testData = {
+                //         Source                                        Expected Result
+                //
+                {null, "123"},
+                {"100.5", "100.5"},
+                {"0", "0"},
+
+                {"junk", "123"},
+                {"", "123"},
+                {"null", "123"}
+        };
+
+        for (String[] aTestData : testData) {
+            String source = aTestData[0];
+            Double expected = (aTestData[1] != null) ? new Double(aTestData[1]) : null;
+
+            TestCase.assertEquals("Validating double '" + source + "'", expected, xssAPI.getValidDouble(source, 123));
+        }
+    }
+
     @Test
     public void testGetValidDimension() {
         String[][] testData = {
@@ -378,10 +404,13 @@ public void testEncodeForJSString() {
                 {null, null},
                 {"simple", "simple"},
 
-                {"break\"out", "break\\\"out"},
-                {"break'out", "break\\'out"},
-                {"'alert(document.cookie)", "\\'alert(document.cookie)"},
-                {"2014-04-22T10:11:24.002+01:00", "2014-04-22T10:11:24.002+01:00"}
+                {"break\"out", "break\\x22out"},
+                {"break'out", "break\\x27out"},
+
+                {"</script>", "<\\/script>"},
+
+                {"'alert(document.cookie)", "\\x27alert(document.cookie)"},
+                {"2014-04-22T10:11:24.002+01:00", "2014\\u002D04\\u002D22T10:11:24.002+01:00"}
         };
 
         for (String[] aTestData : testData) {
@@ -408,7 +437,7 @@ public void testGetValidJSToken() {
                 {"\"literal string\"", "\"literal string\""},
                 {"'literal string'", "'literal string'"},
                 {"\"bad literal'", RUBBISH},
-                {"'literal'); junk'", "'literal\\'); junk'"},
+                {"'literal'); junk'", "'literal\\x27); junk'"},
 
                 {"1200", "1200"},
                 {"3.14", "3.14"},
@@ -648,6 +677,10 @@ public void testGetValidXML() {
                 {
                         "<t><w>xyz</t></w>",
                         RUBBISH_XML
+                },
+                {
+                        "<?xml version=\"1.0\"?><!DOCTYPE test SYSTEM \"http://nonExistentHost:1234/\"><test/>",
+                        "<?xml version=\"1.0\"?><!DOCTYPE test SYSTEM \"http://nonExistentHost:1234/\"><test/>"
                 }
         };
         for (String[] aTestData : testData) {