From 7e6f641ebb142663cbd1653dc49bed725edf7f56 Mon Sep 17 00:00:00 2001 From: Maurizio Cucchiara Date: Thu, 18 Apr 2013 09:49:41 +0000 Subject: [PATCH] Disable eval expressions git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/branches/STRUTS_2_3_14_X@1469249 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/struts2/StrutsConstants.java | 3 ++ .../struts2/config/BeanSelectionProvider.java | 1 + .../opensymphony/xwork2/XWorkConstants.java | 1 + .../config/impl/DefaultConfiguration.java | 1 + .../providers/XWorkConfigurationProvider.java | 1 + .../opensymphony/xwork2/ognl/OgnlUtil.java | 52 ++++++++++++++----- 6 files changed, 46 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index d4665d778..8a6810921 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -228,6 +228,9 @@ public final class StrutsConstants { /** Enables caching of parsed OGNL expressions **/ public static final String STRUTS_ENABLE_OGNL_EXPRESSION_CACHE = "struts.ognl.enableExpressionCache"; + /** Enables evaluation of OGNL expressions **/ + public static final String STRUTS_ENABLE_OGNL_EVAL_EXPRESSION = "struts.ognl.enableOGNLEvalExpression"; + /** The{@link org.apache.struts2.views.util.UrlHelper} implementation class **/ public static final String STRUTS_URL_HELPER = "struts.view.urlHelper"; diff --git a/core/src/main/java/org/apache/struts2/config/BeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/BeanSelectionProvider.java index e3cf0db6f..5b0bb3b61 100644 --- a/core/src/main/java/org/apache/struts2/config/BeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/BeanSelectionProvider.java @@ -375,6 +375,7 @@ public class BeanSelectionProvider implements ConfigurationProvider { // Convert Struts properties into XWork properties convertIfExist(props, StrutsConstants.STRUTS_LOG_MISSING_PROPERTIES, XWorkConstants.LOG_MISSING_PROPERTIES); convertIfExist(props, StrutsConstants.STRUTS_ENABLE_OGNL_EXPRESSION_CACHE, XWorkConstants.ENABLE_OGNL_EXPRESSION_CACHE); + convertIfExist(props, StrutsConstants.STRUTS_ENABLE_OGNL_EVAL_EXPRESSION, XWorkConstants.ENABLE_OGNL_EVAL_EXPRESSION); convertIfExist(props, StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, XWorkConstants.ALLOW_STATIC_METHOD_ACCESS); convertIfExist(props, StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, XWorkConstants.RELOAD_XML_CONFIGURATION); diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java index 17ffbebad..19363680e 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java @@ -13,6 +13,7 @@ public final class XWorkConstants { public static final String DEV_MODE = "devMode"; public static final String LOG_MISSING_PROPERTIES = "logMissingProperties"; public static final String ENABLE_OGNL_EXPRESSION_CACHE = "enableOGNLExpressionCache"; + public static final String ENABLE_OGNL_EVAL_EXPRESSION = "enableOGNLEvalExpression"; public static final String RELOAD_XML_CONFIGURATION = "reloadXmlConfiguration"; public static final String ALLOW_STATIC_METHOD_ACCESS = "allowStaticMethodAccess"; public static final String XWORK_LOGGER_FACTORY = "xwork.loggerFactory"; diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/xwork-core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 911c5a331..f2a026938 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -319,6 +319,7 @@ public class DefaultConfiguration implements Configuration { builder.factory(OgnlUtil.class, Scope.SINGLETON); builder.constant(XWorkConstants.DEV_MODE, "false"); builder.constant(XWorkConstants.LOG_MISSING_PROPERTIES, "false"); + builder.constant(XWorkConstants.ENABLE_OGNL_EVAL_EXPRESSION, "false"); builder.constant(XWorkConstants.RELOAD_XML_CONFIGURATION, "false"); return builder.create(true); } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java index 495165093..d11a8e5c8 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java @@ -151,6 +151,7 @@ public class XWorkConfigurationProvider implements ConfigurationProvider { props.setProperty(XWorkConstants.DEV_MODE, Boolean.FALSE.toString()); props.setProperty(XWorkConstants.LOG_MISSING_PROPERTIES, Boolean.FALSE.toString()); props.setProperty(XWorkConstants.ENABLE_OGNL_EXPRESSION_CACHE, Boolean.TRUE.toString()); + props.setProperty(XWorkConstants.ENABLE_OGNL_EVAL_EXPRESSION, Boolean.FALSE.toString()); props.setProperty(XWorkConstants.RELOAD_XML_CONFIGURATION, Boolean.FALSE.toString()); } 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 a2ae49058..f488ad516 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 @@ -56,6 +56,7 @@ public class OgnlUtil { private TypeConverter defaultConverter; static boolean devMode = false; static boolean enableExpressionCache = true; + private static boolean enableEvalExpression; @Inject public void setXWorkConverter(XWorkConverter conv) { @@ -67,11 +68,20 @@ public class OgnlUtil { devMode = "true".equals(mode); } - @Inject("enableOGNLExpressionCache") + @Inject(XWorkConstants.ENABLE_OGNL_EXPRESSION_CACHE) public static void setEnableExpressionCache(String cache) { enableExpressionCache = "true".equals(cache); } + @Inject(value = XWorkConstants.ENABLE_OGNL_EVAL_EXPRESSION, required = false) + public void setEnableEvalExpression(String evalExpression) { + enableEvalExpression = "true".equals(evalExpression); + if(enableEvalExpression){ + LOG.warn("Enabling OGNL expression evaluation may introduce security risks " + + "(see http://struts.apache.org/release/2.3.x/docs/s2-013.html for further details)"); + } + } + /** * Sets the object's properties using the default type converter, defaulting to not throw * exceptions for problems setting the properties. @@ -217,7 +227,7 @@ public class OgnlUtil { } protected void setValue(String name, Map context, Object root, Object value, boolean evalName) throws OgnlException { - Object tree = compile(name); + Object tree = compile(name, context); if (!evalName && isEvalExpression(tree, context)) { throw new OgnlException("Eval expression cannot be used as parameter name"); } @@ -227,30 +237,46 @@ public class OgnlUtil { private boolean isEvalExpression(Object tree, Map context) throws OgnlException { if (tree instanceof SimpleNode) { SimpleNode node = (SimpleNode) tree; - return node.isEvalChain((OgnlContext) context); + OgnlContext ognlContext = null; + + if (context!=null && context instanceof OgnlContext) { + ognlContext = (OgnlContext) context; + } + return node.isEvalChain(ognlContext); } return false; } public Object getValue(String name, Map context, Object root) throws OgnlException { - return Ognl.getValue(compile(name), context, root); + return Ognl.getValue(compile(name, context), context, root); } public Object getValue(String name, Map context, Object root, Class resultType) throws OgnlException { - return Ognl.getValue(compile(name), context, root, resultType); + return Ognl.getValue(compile(name, context), context, root, resultType); } public Object compile(String expression) throws OgnlException { + return compile(expression, null); + } + + public Object compile(String expression, Map context) throws OgnlException { + Object tree; if (enableExpressionCache) { - Object o = expressions.get(expression); - if (o == null) { - o = Ognl.parseExpression(expression); - expressions.putIfAbsent(expression, o); + tree = expressions.get(expression); + if (tree == null) { + tree = Ognl.parseExpression(expression); + expressions.putIfAbsent(expression, tree); } - return o; - } else - return Ognl.parseExpression(expression); + } else { + tree = Ognl.parseExpression(expression); + } + + if (!enableEvalExpression && isEvalExpression(tree, context)) { + throw new OgnlException("Eval expressions has been disabled"); + } + + return tree; } /** @@ -312,7 +338,7 @@ public class OgnlUtil { PropertyDescriptor toPd = toPdHash.get(fromPd.getName()); if ((toPd != null) && (toPd.getWriteMethod() != null)) { try { - Object expr = compile(fromPd.getName()); + Object expr = compile(fromPd.getName(), context); Object value = Ognl.getValue(expr, contextFrom, from); Ognl.setValue(expr, contextTo, to, value); } catch (OgnlException e) {