Merge pull request #182 from coheigea/SHIRO-731
SHIRO-731 - Use OWasp Java Encoder to escape user supplied content to…
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 @@
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.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 @@
*/
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 @@
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 @@
// 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 @@
// 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 @@
* @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.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 @@
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);
