You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2014/04/06 16:03:37 UTC
[24/50] [abbrv] git commit: WW-4146 Caches only valid Ognl
expressions to avoid cache attack
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 <lu...@apache.org>
Authored: Thu Mar 27 19:07:11 2014 +0100
Committer: Lukasz Lenart <lu...@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;