CVE-2016-4433

@@ -427,13 +427,13 @@ protected String invokeAction(Object action, ActionConfig actionConfig) throws E
 
             Object methodResult;
             try {
-                methodResult = ognlUtil.getValue(methodName + "()", getStack().getContext(), action);
+                methodResult = ognlUtil.callMethod(methodName + "()", getStack().getContext(), action);
             } catch (MethodFailedException e) {
                 // if reason is missing method, try find version with "do" prefix
                 if (e.getReason() instanceof NoSuchMethodException) {
                     try {
                         String altMethodName = "do" + methodName.substring(0, 1).toUpperCase() + methodName.substring(1) + "()";
-                        methodResult = ognlUtil.getValue(altMethodName, getStack().getContext(), action);
+                        methodResult = ognlUtil.callMethod(altMethodName, getStack().getContext(), action);
                     } catch (MethodFailedException e1) {
                         // if still method doesn't exist, try checking UnknownHandlers
                         if (e1.getReason() instanceof NoSuchMethodException) {

@@ -290,6 +290,9 @@ public Void execute(Object tree) throws OgnlException {
                 if (isEvalExpression(tree, context)) {
                     throw new OgnlException("Eval expression/chained expressions cannot be used as parameter name");
                 }
+                if (isArithmeticExpression(tree, context)) {
+                    throw new OgnlException("Arithmetic expressions cannot be used as parameter name");
+                }
                 Ognl.setValue(tree, context, root, value);
                 return null;
             }
@@ -309,6 +312,32 @@ private boolean isEvalExpression(Object tree, Map<String, Object> context) throw
         return false;
     }
 
+    private boolean isArithmeticExpression(Object tree, Map<String, Object> context) throws OgnlException {
+        if (tree instanceof SimpleNode) {
+            SimpleNode node = (SimpleNode) tree;
+            OgnlContext ognlContext = null;
+
+            if (context!=null && context instanceof OgnlContext) {
+                ognlContext = (OgnlContext) context;
+            }
+            return node.isOperation(ognlContext);
+        }
+        return false;
+    }
+
+    private boolean isSimpleMethod(Object tree, Map<String, Object> context) throws OgnlException {
+        if (tree instanceof SimpleNode) {
+            SimpleNode node = (SimpleNode) tree;
+            OgnlContext ognlContext = null;
+
+            if (context!=null && context instanceof OgnlContext) {
+                ognlContext = (OgnlContext) context;
+            }
+            return node.isSimpleMethod(ognlContext) && !node.isChain(ognlContext);
+        }
+        return false;
+    }
+
     public Object getValue(final String name, final Map<String, Object> context, final Object root) throws OgnlException {
         return compileAndExecute(name, context, new OgnlTask<Object>() {
             public Object execute(Object tree) throws OgnlException {
@@ -317,6 +346,14 @@ public Object execute(Object tree) throws OgnlException {
         });
     }
 
+    public Object callMethod(final String name, final Map<String, Object> context, final Object root) throws OgnlException {
+        return compileAndExecuteMethod(name, context, new OgnlTask<Object>() {
+            public Object execute(Object tree) throws OgnlException {
+                return Ognl.getValue(tree, context, root);
+            }
+        });
+    }
+
     public Object getValue(final String name, final Map<String, Object> context, final Object root, final Class resultType) throws OgnlException {
         return compileAndExecute(name, context, new OgnlTask<Object>() {
             public Object execute(Object tree) throws OgnlException {
@@ -351,6 +388,27 @@ private <T> Object compileAndExecute(String expression, Map<String, Object> cont
         return exec;
     }
 
+    private <T> Object compileAndExecuteMethod(String expression, Map<String, Object> context, OgnlTask<T> task) throws OgnlException {
+        Object tree;
+        if (enableExpressionCache) {
+            tree = expressions.get(expression);
+            if (tree == null) {
+                tree = Ognl.parseExpression(expression);
+                checkSimpleMethod(tree, context);
+            }
+        } else {
+            tree = Ognl.parseExpression(expression);
+            checkSimpleMethod(tree, context);
+        }
+
+        final T exec = task.execute(tree);
+        // if cache is enabled and it's a valid expression, puts it in
+        if(enableExpressionCache) {
+            expressions.putIfAbsent(expression, tree);
+        }
+        return exec;
+    }
+
     public Object compile(String expression, Map<String, Object> context) throws OgnlException {
         return compileAndExecute(expression,context,new OgnlTask<Object>() {
             public Object execute(Object tree) throws OgnlException {
@@ -365,6 +423,12 @@ private void checkEnableEvalExpression(Object tree, Map<String, Object> context)
         }
     }
 
+    private void checkSimpleMethod(Object tree, Map<String, Object> context) throws OgnlException {
+        if (!isSimpleMethod(tree, context)) {
+            throw new OgnlException("It isn't a simple method which can be called!");
+        }
+    }
+
     /**
      * Copies the properties in the object "from" and sets them in the object "to"
      * using specified type converter, or {@link com.opensymphony.xwork2.conversion.impl.XWorkConverter} if none

@@ -765,6 +765,21 @@ public void testBlockSequenceOfExpressions() throws Exception {
         assertEquals(expected.getMessage(), "Eval expressions/chained expressions have been disabled!");
     }
 
+    public void testCallMethod() throws Exception {
+        Foo foo = new Foo();
+
+        Exception expected = null;
+        try {
+            ognlUtil.callMethod("#booScope=@myclass@DEFAULT_SCOPE,#bootScope.init()", ognlUtil.createDefaultContext(foo), foo);
+            fail();
+        } catch (OgnlException e) {
+            expected = e;
+        }
+        assertNotNull(expected);
+        assertSame(OgnlException.class, expected.getClass());
+        assertEquals(expected.getMessage(), "It isn't a simple method which can be called!");
+    }
+
     public static class Email {
         String address;