Repository: struts Updated Branches: refs/heads/master 43c030941 -> 48ea26be7
Solves issue with vulnerable parameters Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/f420f284 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/f420f284 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/f420f284 Branch: refs/heads/master Commit: f420f28466cb82915defc4e12466b298c275abaf Parents: 925741a Author: Lukasz Lenart <lukaszlen...@apache.org> Authored: Tue Sep 22 07:24:49 2015 +0200 Committer: Lukasz Lenart <lukaszlen...@apache.org> Committed: Tue Sep 22 07:24:49 2015 +0200 ---------------------------------------------------------------------- .../DefaultExcludedPatternsChecker.java | 2 +- .../interceptor/ParametersInterceptorTest.java | 6 ++-- .../DefaultExcludedPatternsCheckerTest.java | 35 ++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/f420f284/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java ---------------------------------------------------------------------- diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java index d96b67a..93d72ca 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java @@ -16,7 +16,7 @@ public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker { private static final Logger LOG = LoggerFactory.getLogger(DefaultExcludedPatternsChecker.class); public static final String[] EXCLUDED_PATTERNS = { - "(^|.*#)(dojo|struts|session|request|application|servlet(Request|Response)|parameters|context|_memberAccess)(\\.|\\[).*", + "(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*", "^(action|method):.*" }; http://git-wip-us.apache.org/repos/asf/struts/blob/f420f284/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java ---------------------------------------------------------------------- diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java index f20e178..5dcc3e0 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java @@ -110,11 +110,13 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(action, vs, params); // then - assertEquals(1, action.getActionMessages().size()); + assertEquals(2, action.getActionMessages().size()); String msg1 = action.getActionMessage(0); + String msg2 = action.getActionMessage(1); - assertTrue(msg1.contains("Error setting expression 'top['name'](0)' with value 'true'")); + assertEquals("Error setting expression 'name' with value '(#context[\"xwork.MethodAccessor.denyMethodExecution\"]= new java.lang.Boolean(false), #_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWNAGE'))(meh)'", msg1); + assertEquals("Error setting expression 'top['name'](0)' with value 'true'", msg2); assertNull(action.getName()); } http://git-wip-us.apache.org/repos/asf/struts/blob/f420f284/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java ---------------------------------------------------------------------- diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java index 22e4a73..d917808 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java @@ -3,6 +3,7 @@ package com.opensymphony.xwork2.security; import com.opensymphony.xwork2.XWorkTestCase; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase { @@ -35,6 +36,10 @@ public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase { add("%{#servletResponse.test}"); add("%{#ServletResponse['test']}"); add("%{#ServletResponse.test}"); + add("%{#servletContext['test']}"); + add("%{#servletContext.test}"); + add("%{#ServletContext['test']}"); + add("%{#ServletContext.test}"); add("%{#parameters['test']}"); add("%{#parameters.test}"); add("%{#Parameters['test']}"); @@ -65,6 +70,36 @@ public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase { } } + public void testDefaultExcludePatterns() throws Exception { + // given + List<String> prefixes = Arrays.asList("#[0].%s", "[0].%s", "top.%s", "%{[0].%s}", "%{#[0].%s}", "%{top.%s}", "%{#top.%s}", "%{#%s}", "%{%s}", "#%s"); + List<String> inners = Arrays.asList("servletRequest", "servletResponse", "servletContext", "application", "session", "struts", "request", "response", "dojo", "parameters"); + List<String> suffixes = Arrays.asList("['test']", "[\"test\"]", ".test"); + + DefaultExcludedPatternsChecker checker = new DefaultExcludedPatternsChecker(); + checker.setAdditionalExcludePatterns(".*(^|\\.|\\[|'|\")class(\\.|\\[|'|\").*"); + + List<String> params = new ArrayList<String>(); + for (String prefix : prefixes) { + for (String inner : inners) { + String innerUp = inner.substring(0, 1).toUpperCase() + inner.substring(1); + for (String suffix : suffixes) { + params.add(prefix.replace("%s", inner + suffix)); + params.add(prefix.replace("%s", innerUp + suffix)); + } + } + } + + for (String param : params) { + System.out.println(param); + // when + ExcludedPatternsChecker.IsExcluded actual = checker.isExcluded(param); + + // then + assertTrue("Access to " + param + " is possible!", actual.isExcluded()); + } + } + public void testParamWithClassInName() throws Exception { // given List<String> properParams = new ArrayList<String>();
