You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by fp...@apache.org on 2019/11/19 19:30:11 UTC

[shiro] branch master updated: SHIRO-731 - Use OWasp Java Encoder to escape user supplied content to the logs

This is an automated email from the ASF dual-hosted git repository.

fpapon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shiro.git


The following commit(s) were added to refs/heads/master by this push:
     new 75dec95  SHIRO-731 - Use OWasp Java Encoder to escape user supplied content to the logs
     new fa1686d  Merge pull request #182 from coheigea/SHIRO-731
75dec95 is described below

commit 75dec95d67f6b51bae6cf8ca4c9e8207fd298418
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Tue Nov 19 16:52:55 2019 +0000

    SHIRO-731 - Use OWasp Java Encoder to escape user supplied content to the logs
---
 NOTICE                                                             | 4 ++--
 pom.xml                                                            | 7 +++++++
 support/features/src/main/resources/features.xml                   | 1 +
 web/pom.xml                                                        | 4 ++++
 .../main/java/org/apache/shiro/web/filter/PathMatchingFilter.java  | 3 ++-
 .../shiro/web/filter/mgt/PathMatchingFilterChainResolver.java      | 4 +++-
 web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java   | 6 ++++--
 web/src/main/java/org/apache/shiro/web/util/RedirectView.java      | 6 +++---
 web/src/main/java/org/apache/shiro/web/util/WebUtils.java          | 3 ++-
 9 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/NOTICE b/NOTICE
index eea43c6..088a14d 100644
--- a/NOTICE
+++ b/NOTICE
@@ -1,5 +1,5 @@
 Apache Shiro
-Copyright 2008-2012 The Apache Software Foundation
+Copyright 2008-2019 The Apache Software Foundation
 
 This product includes software developed at
 The Apache Software Foundation (http://www.apache.org/).
@@ -12,4 +12,4 @@ with continued modifications.
 Certain parts (StringUtils etc.) of the source code for this 
 product was copied for simplicity and to reduce dependencies 
 from the source code developed by the Spring Framework Project 
-(http://www.springframework.org).
\ No newline at end of file
+(http://www.springframework.org).
diff --git a/pom.xml b/pom.xml
index 407e06c..ae1e03a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -94,6 +94,7 @@
         <javax.annotation.api.version>1.3.2</javax.annotation.api.version>
         <jdk.version>1.8</jdk.version>
         <jetty.version>9.4.20.v20190813</jetty.version>
+        <owasp.java.encoder.version>1.2.2</owasp.java.encoder.version>
         <!-- Don't change this version without also changing the shiro-quartz and shiro-features
              modules' OSGi metadata: -->
         <quartz.version>2.3.2</quartz.version>
@@ -861,6 +862,12 @@
             </dependency>
 
             <dependency>
+                <groupId>org.owasp.encoder</groupId>
+                <artifactId>encoder</artifactId>
+                <version>${owasp.java.encoder.version}</version>
+            </dependency>
+
+            <dependency>
                 <groupId>ch.qos.logback</groupId>
                 <artifactId>logback-classic</artifactId>
                 <version>${logback.version}</version>
diff --git a/support/features/src/main/resources/features.xml b/support/features/src/main/resources/features.xml
index ecb4628..0196f23 100644
--- a/support/features/src/main/resources/features.xml
+++ b/support/features/src/main/resources/features.xml
@@ -28,6 +28,7 @@
     <feature name="shiro-web" version="${project.version}">
         <feature version="${project.version}">shiro-core</feature>
         <feature version="[2,5)">war</feature>
+        <bundle>wrap:mvn:org.owasp.encoder/encoder/${owasp.java.encoder.version}</bundle>
         <bundle>mvn:org.apache.shiro/shiro-web/${project.version}</bundle>
     </feature>
 
diff --git a/web/pom.xml b/web/pom.xml
index 2df91e0..0cf06c1 100644
--- a/web/pom.xml
+++ b/web/pom.xml
@@ -53,6 +53,10 @@
             <groupId>javax.servlet</groupId>
             <artifactId>javax.servlet-api</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.owasp.encoder</groupId>
+            <artifactId>encoder</artifactId>
+        </dependency>
         <!-- Test dependencies - scope set appropriately already in the parent pom-->
         <dependency>
             <groupId>org.apache.shiro</groupId>
diff --git a/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java b/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
index aa3b679..bd8f879 100644
--- a/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
+++ b/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
@@ -23,6 +23,7 @@ import org.apache.shiro.util.PatternMatcher;
 import org.apache.shiro.util.StringUtils;
 import org.apache.shiro.web.servlet.AdviceFilter;
 import org.apache.shiro.web.util.WebUtils;
+import org.owasp.encoder.Encode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -120,7 +121,7 @@ public abstract class PathMatchingFilter extends AdviceFilter implements PathCon
      */
     protected boolean pathsMatch(String path, ServletRequest request) {
         String requestURI = getPathWithinApplication(request);
-        log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, requestURI);
+        log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(requestURI));
         return pathsMatch(path, requestURI);
     }
 
diff --git a/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java b/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
index bb70885..a0b9898 100644
--- a/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
+++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
@@ -21,6 +21,8 @@ package org.apache.shiro.web.filter.mgt;
 import org.apache.shiro.util.AntPathMatcher;
 import org.apache.shiro.util.PatternMatcher;
 import org.apache.shiro.web.util.WebUtils;
+import org.owasp.encoder.Encode;
+import org.owasp.encoder.Encoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -105,7 +107,7 @@ public class PathMatchingFilterChainResolver implements FilterChainResolver {
             // If the path does match, then pass on to the subclass implementation for specific checks:
             if (pathMatches(pathPattern, requestURI)) {
                 if (log.isTraceEnabled()) {
-                    log.trace("Matched path pattern [" + pathPattern + "] for requestURI [" + requestURI + "].  " +
+                    log.trace("Matched path pattern [" + pathPattern + "] for requestURI [" + Encode.forHtml(requestURI) + "].  " +
                             "Utilizing corresponding filter chain...");
                 }
                 return filterChainManager.proxy(originalChain, pathPattern);
diff --git a/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java b/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java
index 7022586..d28405c 100644
--- a/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java
+++ b/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java
@@ -19,6 +19,7 @@
 package org.apache.shiro.web.servlet;
 
 import org.apache.shiro.util.StringUtils;
+import org.owasp.encoder.Encode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -412,10 +413,11 @@ public class SimpleCookie implements Cookie {
             // Validate that the cookie is used at the correct place.
             String path = StringUtils.clean(getPath());
             if (path != null && !pathMatches(path, request.getRequestURI())) {
-                log.warn("Found '{}' cookie at path '{}', but should be only used for '{}'", new Object[] { name, request.getRequestURI(), path});
+                log.warn("Found '{}' cookie at path '{}', but should be only used for '{}'", 
+                		new Object[] { name, Encode.forHtml(request.getRequestURI()), path});
             } else {
                 value = cookie.getValue();
-                log.debug("Found '{}' cookie value [{}]", name, value);
+                log.debug("Found '{}' cookie value [{}]", name, Encode.forHtml(value));
             }
         } else {
             log.trace("No '{}' cookie value", name);
diff --git a/web/src/main/java/org/apache/shiro/web/util/RedirectView.java b/web/src/main/java/org/apache/shiro/web/util/RedirectView.java
index 8e039b1..4a17a2f 100644
--- a/web/src/main/java/org/apache/shiro/web/util/RedirectView.java
+++ b/web/src/main/java/org/apache/shiro/web/util/RedirectView.java
@@ -292,16 +292,16 @@ public class RedirectView {
      * @param http10Compatible whether to stay compatible with HTTP 1.0 clients
      * @throws IOException if thrown by response methods
      */
-    @SuppressWarnings({"UnusedDeclaration"})
     protected void sendRedirect(HttpServletRequest request, HttpServletResponse response,
                                 String targetUrl, boolean http10Compatible) throws IOException {
+        String encodedRedirectURL = response.encodeRedirectURL(targetUrl);
         if (http10Compatible) {
             // Always send status code 302.
-            response.sendRedirect(response.encodeRedirectURL(targetUrl));
+            response.sendRedirect(encodedRedirectURL);
         } else {
             // Correct HTTP status code is 303, in particular for POST requests.
             response.setStatus(303);
-            response.setHeader("Location", response.encodeRedirectURL(targetUrl));
+            response.setHeader("Location", encodedRedirectURL);
         }
     }
 
diff --git a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
index 3f3f750..c61d244 100644
--- a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
+++ b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
@@ -26,6 +26,7 @@ import org.apache.shiro.util.StringUtils;
 import org.apache.shiro.web.env.EnvironmentLoader;
 import org.apache.shiro.web.env.WebEnvironment;
 import org.apache.shiro.web.filter.AccessControlFilter;
+import org.owasp.encoder.Encode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -347,7 +348,7 @@ public class WebUtils {
             return URLDecoder.decode(source, enc);
         } catch (UnsupportedEncodingException ex) {
             if (log.isWarnEnabled()) {
-                log.warn("Could not decode request string [" + source + "] with encoding '" + enc +
+                log.warn("Could not decode request string [" + Encode.forHtml(source) + "] with encoding '" + Encode.forHtml(enc) +
                         "': falling back to platform default encoding; exception message: " + ex.getMessage());
             }
             return URLDecoder.decode(source);