Skip to content
Permalink
Browse files

Improves pattern to avoid classloader pollution and adds dedicated tests

  • Loading branch information
lukaszlenart committed Mar 30, 2014
1 parent 9a94699 commit aaf5a3010e3c11ae14e3d3c966a53ebab67146be
@@ -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">
@@ -87,7 +87,7 @@ public boolean acceptableParameterName(String parameterName) {
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 void testInsecureParamaters() throws Exception {
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");

4 comments on commit aaf5a30

@zdonking

This comment has been minimized.

Copy link

zdonking replied Apr 25, 2014

why so many bugs

@lukaszlenart

This comment has been minimized.

Copy link
Member Author

lukaszlenart replied Apr 25, 2014

Don't get what you mean :(

Or maybe because too many people is using Struts, the same case as with M$ Windows ;-)

@leexiaodong

This comment has been minimized.

Copy link

leexiaodong replied Apr 25, 2014

why strtus2 aways have some leak??

@lukaszlenart

This comment has been minimized.

Copy link
Member Author

lukaszlenart replied Apr 25, 2014

point me to code that doesn't? ;-)

Please sign in to comment.
You can’t perform that action at this time.