Repository: struts Updated Branches: refs/heads/develop 9a94699da -> aaf5a3010
Improves pattern to avoid classloader pollution and adds dedicated tests Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/aaf5a301 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/aaf5a301 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/aaf5a301 Branch: refs/heads/develop Commit: aaf5a3010e3c11ae14e3d3c966a53ebab67146be Parents: 9a94699 Author: Lukasz Lenart <lukaszlen...@apache.org> Authored: Sun Mar 30 21:27:05 2014 +0200 Committer: Lukasz Lenart <lukaszlen...@apache.org> Committed: Sun Mar 30 21:27:05 2014 +0200 ---------------------------------------------------------------------- core/src/main/resources/struts-default.xml | 8 +- .../interceptor/ParametersInterceptorTest.java | 86 +++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/aaf5a301/core/src/main/resources/struts-default.xml ---------------------------------------------------------------------- diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 5c446b1..87f1ff5 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -203,7 +203,7 @@ <interceptor-ref name="multiselect"/> <interceptor-ref name="actionMappingParams"/> <interceptor-ref name="params"> - <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> + <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> </interceptor-ref> <interceptor-ref name="conversionError"/> <interceptor-ref name="deprecation"/> @@ -260,7 +260,7 @@ <interceptor-ref name="datetime"/> <interceptor-ref name="multiselect"/> <interceptor-ref name="params"> - <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> + <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> </interceptor-ref> <interceptor-ref name="servletConfig"/> <interceptor-ref name="prepare"/> @@ -270,7 +270,7 @@ <interceptor-ref name="staticParams"/> <interceptor-ref name="actionMappingParams"/> <interceptor-ref name="params"> - <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> + <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> </interceptor-ref> <interceptor-ref name="conversionError"/> <interceptor-ref name="validation"> @@ -308,7 +308,7 @@ <interceptor-ref name="staticParams"/> <interceptor-ref name="actionMappingParams"/> <interceptor-ref name="params"> - <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> + <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param> </interceptor-ref> <interceptor-ref name="conversionError"/> <interceptor-ref name="validation"> http://git-wip-us.apache.org/repos/asf/struts/blob/aaf5a301/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 50eeb4f..5a4485d 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 @@ -87,7 +87,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { assertEquals(expected, actual); } - public void testInsecureParamaters() throws Exception { + public void testInsecureParameters() throws Exception { // given loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml")); final Map<String, Object> params = new HashMap<String, Object>() { @@ -118,6 +118,90 @@ public class ParametersInterceptorTest extends XWorkTestCase { assertNull(action.getName()); } + public void testClassPollutionBlockedByPattern() throws Exception { + // given + final String pollution1 = "class.classLoader.jarPath"; + final String pollution2 = "model.class.classLoader.jarPath"; + + loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml")); + final Map<String, Object> params = new HashMap<String, Object>() { + { + put(pollution1, "bad"); + put(pollution2, "very bad"); + } + }; + + final Map<String, Boolean> excluded = new HashMap<String, Boolean>(); + ParametersInterceptor pi = new ParametersInterceptor() { + + @Override + protected boolean isExcluded(String paramName) { + boolean result = super.isExcluded(paramName); + excluded.put(paramName, result); + return result; + } + + }; + + pi.setExcludeParams("(.*\\.|^)class\\..*"); + container.inject(pi); + ValueStack vs = ActionContext.getContext().getValueStack(); + + // when + ValidateAction action = new ValidateAction(); + pi.setParameters(action, vs, params); + + // then + assertEquals(0, action.getActionMessages().size()); + assertTrue(excluded.get(pollution1)); + assertTrue(excluded.get(pollution2)); + } + + public void testClassPollutionBlockedByOgnl() throws Exception { + // given + final String pollution1 = "class.classLoader.jarPath"; + final String pollution2 = "model.class.classLoader.jarPath"; + + loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml")); + final Map<String, Object> params = new HashMap<String, Object>() { + { + put(pollution1, "bad"); + put(pollution2, "very bad"); + } + }; + + final Map<String, Boolean> excluded = new HashMap<String, Boolean>(); + ParametersInterceptor pi = new ParametersInterceptor() { + + @Override + protected boolean isExcluded(String paramName) { + boolean result = super.isExcluded(paramName); + excluded.put(paramName, result); + return result; + } + + }; + + container.inject(pi); + ValueStack vs = ActionContext.getContext().getValueStack(); + + // when + ValidateAction action = new ValidateAction(); + pi.setParameters(action, vs, params); + + // then + assertEquals(2, action.getActionMessages().size()); + + String msg1 = action.getActionMessage(0); + String msg2 = action.getActionMessage(1); + + assertEquals("Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg1); + assertEquals("Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg2); + + assertFalse(excluded.get(pollution1)); + assertFalse(excluded.get(pollution2)); + } + public void testDoesNotAllowMethodInvocations() throws Exception { Map<String, Object> params = new HashMap<String, Object>(); params.put("@java.lang.System@exit(1).dummy", "dumb value");
