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 2019/11/14 15:29:02 UTC

[sling-org-apache-sling-xss] branch master updated: SLING-8845 - URL query parameter values are double-escaped for cases where namespace mangling has to be performed

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 9927ab0  SLING-8845 - URL query parameter values are double-escaped for cases where namespace mangling has to be performed
9927ab0 is described below

commit 9927ab07e3a56b47e272c75933049ac5cfcbbb4f
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Thu Nov 14 16:12:46 2019 +0100

    SLING-8845 - URL query parameter values are double-escaped for cases where namespace mangling has to be performed
    
    * use already encoded query + fragment when creating mangled URLs
---
 .../java/org/apache/sling/xss/impl/XSSAPIImpl.java | 79 +++++++++++-----------
 .../org/apache/sling/xss/impl/XSSAPIImplTest.java  | 12 ++++
 2 files changed, 53 insertions(+), 38 deletions(-)

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 1bd3ae7..4022b8a 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
@@ -18,7 +18,9 @@ package org.apache.sling.xss.impl;
 
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.io.UnsupportedEncodingException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URLDecoder;
 import java.util.HashMap;
 import java.util.Map;
@@ -189,45 +191,41 @@ public class XSSAPIImpl implements XSSAPI {
 
     private static final String MANGLE_NAMESPACE_IN_PREFIX = "/_";
 
-    private String mangleNamespaces(String absPath) {
+    private String mangleNamespaces(String absPath) throws URISyntaxException, UnsupportedEncodingException {
         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();
+        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();
             }
-            if (mangledPath != null) {
-                URI mangledURI = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(),
-                        URLDecoder.decode(mangledPath, "UTF-8"),
-                        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();
+        }
+        if (mangledPath != null) {
+            URI mangledURI = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(),
+                    URLDecoder.decode(mangledPath, "UTF-8"),
+                    uri.getQuery(), uri.getFragment());
+            StringBuilder uriBuilder = new StringBuilder();
+            if (StringUtils.isNotEmpty(mangledURI.getScheme()) && StringUtils.isNotEmpty(mangledURI.getAuthority())) {
+                uriBuilder.append(mangledURI.getScheme()).append("://").append(mangledURI.getRawAuthority());
             }
-        } catch (Exception e) {
-            LOGGER.warn("Invalid URI.", e);
+            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();
         }
         return absPath;
     }
@@ -248,9 +246,14 @@ public class XSSAPIImpl implements XSSAPI {
                     .replaceAll("<", "%3C")
                     .replaceAll("`", "%60")
                     .replaceAll(" ", "%20");
-            encodedUrl = mangleNamespaces(encodedUrl);
-            if (xssFilter.isValidHref(encodedUrl)) {
-                return encodedUrl;
+            try {
+                encodedUrl = mangleNamespaces(encodedUrl);
+                if (xssFilter.isValidHref(encodedUrl)) {
+                    return encodedUrl;
+                }
+            } catch (Throwable t) {
+                LOGGER.warn("Unable to validate URL.", t);
+                LOGGER.debug("Passed URL: {}", url);
             }
         }
         // fall through to empty string
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 6852ea6..e365b57 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
@@ -313,6 +313,18 @@ public class XSSAPIImplTest {
                 {
                         "/content/test/",
                         "/content/test/"
+                },
+                { // namespace mangling + encoded parameter values
+                        "/path/to/page/jcr:content/par?key=%25text",
+                        "/path/to/page/_jcr_content/par?key=%25text"
+                },
+                { // namespace mangling + incorrect escape sequence
+                        "/path/to/page/jcr:content/par?key=%text",
+                        ""
+                },
+                { // incorrect escape sequence
+                        "/path/to/page/_jcr_content/par?key=%text",
+                        ""
                 }
         };