fix the potential threat when use "uri = uri + '/' " to bypassed shiro protect
add shiro 682 utests
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 bd8f879..e507ad3 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
@@ -47,6 +47,8 @@
*/
private static final Logger log = LoggerFactory.getLogger(PathMatchingFilter.class);
+ private static final String DEFAULT_PATH_SEPARATOR = "/";
+
/**
* PatternMatcher used in determining which paths to react to for a given request.
*/
@@ -121,6 +123,12 @@
*/
protected boolean pathsMatch(String path, ServletRequest request) {
String requestURI = getPathWithinApplication(request);
+ if (requestURI != null && requestURI.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ requestURI = requestURI.substring(0, requestURI.length() - 1);
+ }
+ if (path != null && path.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ path = path.substring(0, path.length() - 1);
+ }
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 a0b9898..7adcda7 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
@@ -49,6 +49,8 @@
private PatternMatcher pathMatcher;
+ private static final String DEFAULT_PATH_SEPARATOR = "/";
+
public PathMatchingFilterChainResolver() {
this.pathMatcher = new AntPathMatcher();
this.filterChainManager = new DefaultFilterChainManager();
@@ -100,9 +102,20 @@
String requestURI = getPathWithinApplication(request);
+ // in spring web, the requestURI "/resource/menus" ---- "resource/menus/" bose can access the resource
+ // but the pathPattern match "/resource/menus" can not match "resource/menus/"
+ // user can use requestURI + "/" to simply bypassed chain filter, to bypassed shiro protect
+ if(requestURI != null && requestURI.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ requestURI = requestURI.substring(0, requestURI.length() - 1);
+ }
+
+
//the 'chain names' in this implementation are actually path patterns defined by the user. We just use them
//as the chain name for the FilterChainManager's requirements
for (String pathPattern : filterChainManager.getChainNames()) {
+ if (pathPattern != null && pathPattern.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ pathPattern = pathPattern.substring(0, pathPattern.length() - 1);
+ }
// If the path does match, then pass on to the subclass implementation for specific checks:
if (pathMatches(pathPattern, requestURI)) {
diff --git a/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java
index d5e89a8..116aaae 100644
--- a/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java
+++ b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java
@@ -121,5 +121,47 @@
verify(request);
}
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testPathMatchEEnabled() {
+ expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes();
+ expect(request.getRequestURI()).andReturn("/resource/book").anyTimes();
+ replay(request);
+
+ boolean matchEnabled = filter.pathsMatch("/resource/book", request);
+ assertTrue("PathMatch can match URL end with Separator", matchEnabled);
+ verify(request);
+ }
+
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testPathMatchEndWithUrlSeparatorEnabled() {
+ expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes();
+ expect(request.getRequestURI()).andReturn("/resource/book/").anyTimes();
+ replay(request);
+
+ boolean matchEnabled = filter.pathsMatch("/resource/book", request);
+ assertTrue("PathMatch can match URL end with Separator", matchEnabled);
+ verify(request);
+ }
+
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testPathMatchEndWithMultiUrlSeparatorEnabled() {
+ expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes();
+ expect(request.getRequestURI()).andReturn("/resource/book//").anyTimes();
+ replay(request);
+
+ boolean matchEnabled = filter.pathsMatch("/resource/book", request);
+ assertTrue("PathMatch can match URL end with multi Separator", matchEnabled);
+ verify(request);
+ }
+
}
diff --git a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
index 49dc90c..68a8fa2 100644
--- a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
+++ b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
@@ -163,4 +163,70 @@
assertNull(resolved);
verify(request);
}
+
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testGetChain() {
+ HttpServletRequest request = createNiceMock(HttpServletRequest.class);
+ HttpServletResponse response = createNiceMock(HttpServletResponse.class);
+ FilterChain chain = createNiceMock(FilterChain.class);
+
+ //ensure at least one chain is defined:
+ resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
+
+ expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
+ expect(request.getContextPath()).andReturn("");
+ expect(request.getRequestURI()).andReturn("/resource/book");
+ replay(request);
+
+ FilterChain resolved = resolver.getChain(request, response, chain);
+ assertNotNull(resolved);
+ verify(request);
+ }
+
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testGetChainEndWithUrlSeparator() {
+ HttpServletRequest request = createNiceMock(HttpServletRequest.class);
+ HttpServletResponse response = createNiceMock(HttpServletResponse.class);
+ FilterChain chain = createNiceMock(FilterChain.class);
+
+ //ensure at least one chain is defined:
+ resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
+
+ expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
+ expect(request.getContextPath()).andReturn("");
+ expect(request.getRequestURI()).andReturn("/resource/book/");
+ replay(request);
+
+ FilterChain resolved = resolver.getChain(request, response, chain);
+ assertNotNull(resolved);
+ verify(request);
+ }
+
+ /**
+ * Test asserting <a href="https://issues.apache.org/jira/browse/SHIRO-682">SHIRO-682<a/>.
+ */
+ @Test
+ public void testGetChainEndWithMultiUrlSeparator() {
+ HttpServletRequest request = createNiceMock(HttpServletRequest.class);
+ HttpServletResponse response = createNiceMock(HttpServletResponse.class);
+ FilterChain chain = createNiceMock(FilterChain.class);
+
+ //ensure at least one chain is defined:
+ resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
+
+ expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
+ expect(request.getContextPath()).andReturn("");
+ expect(request.getRequestURI()).andReturn("/resource/book//");
+ replay(request);
+
+ FilterChain resolved = resolver.getChain(request, response, chain);
+ assertNotNull(resolved);
+ verify(request);
+ }
}