struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject [24/50] [abbrv] git commit: WW-4146 Caches only valid Ognl expressions to avoid cache attack
Date Sun, 06 Apr 2014 14:03:37 GMT
WW-4146 Caches only valid Ognl expressions to avoid cache attack


Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/86813c1a
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/86813c1a
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/86813c1a

Branch: refs/heads/master
Commit: 86813c1a7214bc002a5d7ce9981a9ef333e27142
Parents: 5fb27ba
Author: Lukasz Lenart <lukaszlenart@apache.org>
Authored: Thu Mar 27 19:07:11 2014 +0100
Committer: Lukasz Lenart <lukaszlenart@apache.org>
Committed: Thu Mar 27 19:07:11 2014 +0100

----------------------------------------------------------------------
 .../com/opensymphony/xwork2/ognl/OgnlUtil.java  | 85 ++++++++++++++------
 .../ognl/accessor/CompoundRootAccessor.java     | 27 ++++---
 2 files changed, 76 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/struts/blob/86813c1a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
index 3e622d5..fa907e3 100644
--- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -226,12 +226,16 @@ public class OgnlUtil {
         setValue(name, context, root, value, true);
     }
 
-    protected void setValue(String name, Map<String, Object> context, Object root,
Object value, boolean evalName) throws OgnlException {
-        Object tree = compile(name, context);
-        if (!evalName && isEvalExpression(tree, context)) {
-            throw new OgnlException("Eval expression cannot be used as parameter name");
-        }
-        Ognl.setValue(tree, context, root, value);
+    protected void setValue(String name, final Map<String, Object> context, final Object
root, final Object value, final boolean evalName) throws OgnlException {
+        compileAndExecute(name, context, new OgnlTask<Void>() {
+            public Void execute(Object tree) throws OgnlException {
+                if (!evalName && isEvalExpression(tree, context)) {
+                    throw new OgnlException("Eval expression cannot be used as parameter
name");
+                }
+                Ognl.setValue(tree, context, root, value);
+                return null;
+            }
+        });
     }
 
     private boolean isEvalExpression(Object tree, Map<String, Object> context) throws
OgnlException {
@@ -247,12 +251,20 @@ public class OgnlUtil {
         return false;
     }
 
-    public Object getValue(String name, Map<String, Object> context, Object root) throws
OgnlException {
-        return Ognl.getValue(compile(name, context), context, root);
+    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 {
+                return Ognl.getValue(tree, context, root);
+            }
+        });
     }
 
-    public Object getValue(String name, Map<String, Object> context, Object root, Class
resultType) throws OgnlException {
-        return Ognl.getValue(compile(name, context), context, root, resultType);
+    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 {
+                return Ognl.getValue(tree, context, root, resultType);
+            }
+        });
     }
 
 
@@ -260,7 +272,7 @@ public class OgnlUtil {
         return compile(expression, null);
     }
 
-    public Object compile(String expression, Map<String, Object> context) throws OgnlException
{
+    private <T> Object compileAndExecute(String expression, Map<String, Object>
context, OgnlTask<T> task) throws OgnlException {
         Object tree;
         if (enableExpressionCache) {
             tree = expressions.get(expression);
@@ -274,7 +286,19 @@ public class OgnlUtil {
             checkEnableEvalExpression(tree, context);
         }
 
-        return tree;
+
+        final T exec = task.execute(tree);
+        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 {
+                return tree;
+            }
+        });
     }
     
     private void checkEnableEvalExpression(Object tree, Map<String, Object> context)
throws OgnlException {
@@ -295,7 +319,7 @@ public class OgnlUtil {
      * @param inclusions collection of method names to included copying  (can be null)
      *                   note if exclusions AND inclusions are supplied and not null nothing
will get copied.
      */
-    public void copy(Object from, Object to, Map<String, Object> context, Collection<String>
exclusions, Collection<String> inclusions) {
+    public void copy(final Object from, final Object to, final Map<String, Object>
context, Collection<String> exclusions, Collection<String> inclusions) {
         if (from == null || to == null) {
             if (LOG.isWarnEnabled()) {
                 LOG.warn("Attempting to copy from or to a null source. This is illegal and
is bein skipped. This may be due to an error in an OGNL expression, action chaining, or some
other event.");
@@ -305,9 +329,9 @@ public class OgnlUtil {
         }
 
         TypeConverter conv = getTypeConverterFromContext(context);
-        Map contextFrom = Ognl.createDefaultContext(from);
+        final Map contextFrom = Ognl.createDefaultContext(from);
         Ognl.setTypeConverter(contextFrom, conv);
-        Map contextTo = Ognl.createDefaultContext(to);
+        final Map contextTo = Ognl.createDefaultContext(to);
         Ognl.setTypeConverter(contextTo, conv);
 
         PropertyDescriptor[] fromPds;
@@ -342,9 +366,14 @@ public class OgnlUtil {
                     PropertyDescriptor toPd = toPdHash.get(fromPd.getName());
                     if ((toPd != null) && (toPd.getWriteMethod() != null)) {
                         try {
-                            Object expr = compile(fromPd.getName(), context);
-                            Object value = Ognl.getValue(expr, contextFrom, from);
-                            Ognl.setValue(expr, contextTo, to, value);
+                            compileAndExecute(fromPd.getName(), context, new OgnlTask<Object>()
{
+                                public Void execute(Object expr) throws OgnlException {
+                                    Object value = Ognl.getValue(expr, contextFrom, from);
+                                    Ognl.setValue(expr, contextTo, to, value);
+                                    return null;
+                                }
+                            });
+
                         } catch (OgnlException e) {
                             if (LOG.isDebugEnabled()) {
                                 LOG.debug("Got OGNL exception", e);
@@ -409,16 +438,19 @@ public class OgnlUtil {
      * @throws IntrospectionException is thrown if an exception occurs during introspection.
      * @throws OgnlException          is thrown by OGNL if the property value could not be
retrieved
      */
-    public Map getBeanMap(Object source) throws IntrospectionException, OgnlException {
-        Map beanMap = new HashMap();
-        Map sourceMap = Ognl.createDefaultContext(source);
+    public Map<String, Object> getBeanMap(final Object source) throws IntrospectionException,
OgnlException {
+        Map<String, Object> beanMap = new HashMap<String, Object>();
+        final Map sourceMap = Ognl.createDefaultContext(source);
         PropertyDescriptor[] propertyDescriptors = getPropertyDescriptors(source);
         for (PropertyDescriptor propertyDescriptor : propertyDescriptors) {
-            String propertyName = propertyDescriptor.getDisplayName();
+            final String propertyName = propertyDescriptor.getDisplayName();
             Method readMethod = propertyDescriptor.getReadMethod();
             if (readMethod != null) {
-                Object expr = compile(propertyName);
-                Object value = Ognl.getValue(expr, sourceMap, source);
+                final Object value = compileAndExecute(propertyName, null, new OgnlTask<Object>()
{
+                    public Object execute(Object expr) throws OgnlException {
+                        return Ognl.getValue(expr, sourceMap, source);
+                    }
+                });
                 beanMap.put(propertyName, value);
             } else {
                 beanMap.put(propertyName, "There is no read method for " + propertyName);
@@ -485,4 +517,9 @@ public class OgnlUtil {
         */
         return defaultConverter;
     }
+
+    private interface OgnlTask<T> {
+        T execute(Object tree) throws OgnlException;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/struts/blob/86813c1a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
index df038b0..58942fd 100644
--- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
+++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
@@ -17,8 +17,8 @@ package com.opensymphony.xwork2.ognl.accessor;
 
 import com.opensymphony.xwork2.XWorkConstants;
 import com.opensymphony.xwork2.XWorkException;
-import com.opensymphony.xwork2.ognl.OgnlValueStack;
 import com.opensymphony.xwork2.inject.Inject;
+import com.opensymphony.xwork2.ognl.OgnlValueStack;
 import com.opensymphony.xwork2.util.CompoundRoot;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.logging.Logger;
@@ -29,6 +29,8 @@ import java.beans.IntrospectionException;
 import java.beans.PropertyDescriptor;
 import java.util.*;
 
+import static java.lang.String.format;
+import static org.apache.commons.lang3.BooleanUtils.toBoolean;
 
 /**
  * A stack that is able to call methods on objects in the stack.
@@ -55,7 +57,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor,
C
 
     private final static Logger LOG = LoggerFactory.getLogger(CompoundRootAccessor.class);
     private final static Class[] EMPTY_CLASS_ARRAY = new Class[0];
-    private static Map invalidMethods = new HashMap();
+    private static Map<MethodCall, Boolean> invalidMethods = new HashMap<MethodCall,
Boolean>();
 
     static boolean devMode = false;
 
@@ -79,7 +81,8 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor,
C
 
                     return;
                 } else if (o instanceof Map) {
-                    Map<Object, Object> map = (Map) o;
+                    @SuppressWarnings("unchecked")
+                    Map<Object, Object> map = (Map<Object, Object>) o;
                     try {
                         map.put(name, value);
                         return;
@@ -98,14 +101,14 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor,
C
             }
         }
 
-        Boolean reportError = (Boolean) context.get(ValueStack.REPORT_ERRORS_ON_NO_PROP);
-
-        final String msg = "No object in the CompoundRoot has a publicly accessible property
named '" + name + "' (no setter could be found).";
+        boolean reportError = toBoolean((Boolean) context.get(ValueStack.REPORT_ERRORS_ON_NO_PROP));
 
-        if ((reportError != null) && (reportError.booleanValue())) {
-            throw new XWorkException(msg);
-        } else {
-            if (devMode) {
+        if (reportError || devMode) {
+            final String msg = format("No object in the CompoundRoot has a publicly accessible
property named '%s' " +
+                    "(no setter could be found).", name);
+            if (reportError) {
+                throw new XWorkException(msg);
+            } else {
                 LOG.warn(msg);
             }
         }
@@ -118,7 +121,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor,
C
         if (name instanceof Integer) {
             Integer index = (Integer) name;
 
-            return root.cutStack(index.intValue());
+            return root.cutStack(index);
         } else if (name instanceof String) {
             if ("top".equals(name)) {
                 if (root.size() > 0) {
@@ -234,7 +237,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor,
C
 
             if ((argTypes == null) || !invalidMethods.containsKey(mc)) {
                 try {
-                    Object value = OgnlRuntime.callMethod((OgnlContext) context, o, name,
name, objects);
+                    Object value = OgnlRuntime.callMethod((OgnlContext) context, o, name,
objects);
 
                     if (value != null) {
                         return value;


Mime
View raw message