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/20 14:01:08 UTC
[shiro] branch master updated: fix the potential threat when use
"uri = uri + '/' " to bypassed shiro protect add shiro 682 utests
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
View the commit online:
https://github.com/apache/shiro/commit/e61a29cd6ad4724cf9d85c463103c01f6bafdc44
The following commit(s) were added to refs/heads/master by this push:
new e61a29c fix the potential threat when use "uri = uri + '/' " to bypassed shiro protect add shiro 682 utests
new 589f10d Merge pull request #127 from tomsun28/uri-match-vulnerability
e61a29c is described below
commit e61a29cd6ad4724cf9d85c463103c01f6bafdc44
Author: tomsun28 <to...@outlook.com>
AuthorDate: Mon Mar 25 12:47:14 2019 +0800
fix the potential threat when use "uri = uri + '/' " to bypassed shiro protect
add shiro 682 utests
---
.../shiro/web/filter/PathMatchingFilter.java | 8 +++
.../mgt/PathMatchingFilterChainResolver.java | 13 +++++
.../shiro/web/filter/PathMatchingFilterTest.java | 42 ++++++++++++++
.../mgt/PathMatchingFilterChainResolverTest.java | 66 ++++++++++++++++++++++
4 files changed, 129 insertions(+)
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 @@ public abstract class PathMatchingFilter extends AdviceFilter implements PathCon
*/
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 @@ public abstract class PathMatchingFilter extends AdviceFilter implements PathCon
*/
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 @@ public class PathMatchingFilterChainResolver implements FilterChainResolver {
private PatternMatcher pathMatcher;
+ private static final String DEFAULT_PATH_SEPARATOR = "/";
+
public PathMatchingFilterChainResolver() {
this.pathMatcher = new AntPathMatcher();
this.filterChainManager = new DefaultFilterChainManager();
@@ -100,9 +102,20 @@ public class PathMatchingFilterChainResolver implements FilterChainResolver {
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 @@ public class PathMatchingFilterTest {
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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
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);
+ }
}