Merge pull request #747 from apache/WW-5340-ognl-guard

WW-5340 Introducing OGNL Guard

core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java

@@ -18,16 +18,65 @@
  */
 package com.opensymphony.xwork2.config.impl;
 
-import com.opensymphony.xwork2.*;
-import com.opensymphony.xwork2.config.*;
-import com.opensymphony.xwork2.config.entities.*;
+import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.DefaultLocaleProviderFactory;
+import com.opensymphony.xwork2.DefaultTextProvider;
+import com.opensymphony.xwork2.FileManager;
+import com.opensymphony.xwork2.FileManagerFactory;
+import com.opensymphony.xwork2.LocaleProviderFactory;
+import com.opensymphony.xwork2.LocalizedTextProvider;
+import com.opensymphony.xwork2.ObjectFactory;
+import com.opensymphony.xwork2.StrutsTextProviderFactory;
+import com.opensymphony.xwork2.TextProvider;
+import com.opensymphony.xwork2.TextProviderFactory;
+import com.opensymphony.xwork2.config.Configuration;
+import com.opensymphony.xwork2.config.ConfigurationException;
+import com.opensymphony.xwork2.config.ContainerProvider;
+import com.opensymphony.xwork2.config.FileManagerFactoryProvider;
+import com.opensymphony.xwork2.config.FileManagerProvider;
+import com.opensymphony.xwork2.config.PackageProvider;
+import com.opensymphony.xwork2.config.RuntimeConfiguration;
+import com.opensymphony.xwork2.config.entities.ActionConfig;
+import com.opensymphony.xwork2.config.entities.InterceptorMapping;
+import com.opensymphony.xwork2.config.entities.PackageConfig;
+import com.opensymphony.xwork2.config.entities.ResultConfig;
+import com.opensymphony.xwork2.config.entities.ResultTypeConfig;
+import com.opensymphony.xwork2.config.entities.UnknownHandlerConfig;
 import com.opensymphony.xwork2.config.providers.EnvsValueSubstitutor;
 import com.opensymphony.xwork2.config.providers.InterceptorBuilder;
 import com.opensymphony.xwork2.config.providers.ValueSubstitutor;
-import com.opensymphony.xwork2.conversion.*;
-import com.opensymphony.xwork2.conversion.impl.*;
-import com.opensymphony.xwork2.factory.*;
-import com.opensymphony.xwork2.inject.*;
+import com.opensymphony.xwork2.conversion.ConversionAnnotationProcessor;
+import com.opensymphony.xwork2.conversion.ConversionFileProcessor;
+import com.opensymphony.xwork2.conversion.ConversionPropertiesProcessor;
+import com.opensymphony.xwork2.conversion.ObjectTypeDeterminer;
+import com.opensymphony.xwork2.conversion.TypeConverter;
+import com.opensymphony.xwork2.conversion.TypeConverterCreator;
+import com.opensymphony.xwork2.conversion.TypeConverterHolder;
+import com.opensymphony.xwork2.conversion.impl.ArrayConverter;
+import com.opensymphony.xwork2.conversion.impl.CollectionConverter;
+import com.opensymphony.xwork2.conversion.impl.DateConverter;
+import com.opensymphony.xwork2.conversion.impl.DefaultConversionAnnotationProcessor;
+import com.opensymphony.xwork2.conversion.impl.DefaultConversionFileProcessor;
+import com.opensymphony.xwork2.conversion.impl.DefaultObjectTypeDeterminer;
+import com.opensymphony.xwork2.conversion.impl.NumberConverter;
+import com.opensymphony.xwork2.conversion.impl.StringConverter;
+import com.opensymphony.xwork2.conversion.impl.XWorkBasicConverter;
+import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
+import com.opensymphony.xwork2.factory.ActionFactory;
+import com.opensymphony.xwork2.factory.ConverterFactory;
+import com.opensymphony.xwork2.factory.DefaultActionFactory;
+import com.opensymphony.xwork2.factory.DefaultInterceptorFactory;
+import com.opensymphony.xwork2.factory.DefaultResultFactory;
+import com.opensymphony.xwork2.factory.DefaultUnknownHandlerFactory;
+import com.opensymphony.xwork2.factory.InterceptorFactory;
+import com.opensymphony.xwork2.factory.ResultFactory;
+import com.opensymphony.xwork2.factory.StrutsConverterFactory;
+import com.opensymphony.xwork2.factory.UnknownHandlerFactory;
+import com.opensymphony.xwork2.inject.Container;
+import com.opensymphony.xwork2.inject.ContainerBuilder;
+import com.opensymphony.xwork2.inject.Context;
+import com.opensymphony.xwork2.inject.Factory;
+import com.opensymphony.xwork2.inject.Scope;
 import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory;
 import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory;
 import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory;
@@ -36,7 +85,13 @@
 import com.opensymphony.xwork2.ognl.OgnlUtil;
 import com.opensymphony.xwork2.ognl.OgnlValueStackFactory;
 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
-import com.opensymphony.xwork2.util.*;
+import com.opensymphony.xwork2.util.CompoundRoot;
+import com.opensymphony.xwork2.util.OgnlTextParser;
+import com.opensymphony.xwork2.util.PatternMatcher;
+import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider;
+import com.opensymphony.xwork2.util.TextParser;
+import com.opensymphony.xwork2.util.ValueStack;
+import com.opensymphony.xwork2.util.ValueStackFactory;
 import com.opensymphony.xwork2.util.fs.DefaultFileManager;
 import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory;
 import com.opensymphony.xwork2.util.location.LocatableProperties;
@@ -47,10 +102,19 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.conversion.StrutsConversionPropertiesProcessor;
-import org.apache.struts2.conversion.StrutsTypeConverterHolder;
 import org.apache.struts2.conversion.StrutsTypeConverterCreator;
+import org.apache.struts2.conversion.StrutsTypeConverterHolder;
+import org.apache.struts2.ognl.OgnlGuard;
+import org.apache.struts2.ognl.StrutsOgnlGuard;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
 
 
 /**
@@ -301,6 +365,7 @@ protected Container createBootstrapContainer(List<ContainerProvider> providers)
         builder.factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON);
         builder.factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON);
         builder.factory(OgnlUtil.class, Scope.SINGLETON);
+        builder.factory(OgnlGuard.class, StrutsOgnlGuard.class, Scope.SINGLETON);
 
         builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON);
 

core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java

@@ -120,6 +120,8 @@
 import org.apache.struts2.dispatcher.Parameter;
 import org.apache.struts2.interceptor.exec.ExecutorProvider;
 import org.apache.struts2.interceptor.exec.StrutsExecutorProvider;
+import org.apache.struts2.ognl.OgnlGuard;
+import org.apache.struts2.ognl.StrutsOgnlGuard;
 import org.apache.struts2.url.QueryStringBuilder;
 import org.apache.struts2.url.QueryStringParser;
 import org.apache.struts2.url.StrutsQueryStringBuilder;
@@ -227,6 +229,7 @@ public void register(ContainerBuilder builder, LocatableProperties props)
             .factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON)
             .factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON)
             .factory(OgnlUtil.class, Scope.SINGLETON)
+            .factory(OgnlGuard.class, StrutsOgnlGuard.class, Scope.SINGLETON)
             .factory(CollectionConverter.class, Scope.SINGLETON)
             .factory(ArrayConverter.class, Scope.SINGLETON)
             .factory(DateConverter.class, Scope.SINGLETON)

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java

@@ -19,23 +19,23 @@
  * A basic cache interface for use with OGNL processing (such as Expression, BeanInfo).
  * All OGNL caches will have an eviction limit, but setting an extremely high value can
  * simulate an "effectively unlimited" cache.
- * 
+ *
  * @param <Key> The type for the cache key entries
  * @param <Value> The type for the cache value entries
  */
 public interface OgnlCache<Key, Value> {
 
-    public Value get(Key key);
+    Value get(Key key);
 
-    public void put(Key key, Value value);
+    void put(Key key, Value value);
 
-    public void putIfAbsent(Key key, Value value);
+    void putIfAbsent(Key key, Value value);
 
-    public int size();
+    int size();
 
-    public void clear();
+    void clear();
 
-    public int getEvictionLimit();
+    int getEvictionLimit();
 
-    public void setEvictionLimit(int cacheEvictionLimit);
+    void setEvictionLimit(int cacheEvictionLimit);
 }

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java

@@ -20,10 +20,10 @@
 
 /**
  * Default OGNL cache implementation.
- * 
+ *
  * Setting a very high eviction limit simulates an unlimited cache.
  * Setting too low an eviction limit will make the cache ineffective.
- * 
+ *
  * @param <Key> The type for the cache key entries
  * @param <Value> The type for the cache value entries
  */

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java

@@ -22,14 +22,14 @@
 
 /**
  * A basic OGNL LRU cache implementation.
- * 
+ *
  * The implementation utilizes a {@link Collections#synchronizedMap(java.util.Map)}
  * backed by a {@link LinkedHashMap}.  May be replaced by a more efficient implementation in the future.
- * 
+ *
  * Setting too low an eviction limit will produce more overhead than value.
  * Setting too high an eviction limit may also produce more overhead than value.
  * An appropriate eviction limit will need to be determined on an individual application basis.
- * 
+ *
  * @param <Key> The type for the cache key entries
  * @param <Value> The type for the cache value entries
  */

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java

@@ -36,6 +36,8 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.ognl.OgnlGuard;
+import org.apache.struts2.ognl.StrutsOgnlGuard;
 
 import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
@@ -54,8 +56,10 @@
 import static com.opensymphony.xwork2.util.TextParseUtil.commaDelimitedStringToSet;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableSet;
+import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toSet;
 import static org.apache.commons.lang3.StringUtils.strip;
+import static org.apache.struts2.ognl.OgnlGuard.EXPR_BLOCKED;
 
 
 /**
@@ -74,20 +78,21 @@ public class OgnlUtil {
     private final OgnlCache<String, Object> expressionCache;
     private final OgnlCache<Class<?>, BeanInfo> beanInfoCache;
     private TypeConverter defaultConverter;
+    private final OgnlGuard ognlGuard;
 
     private boolean devMode;
     private boolean enableExpressionCache = true;
     private boolean enableEvalExpression;
 
-    private Set<String> excludedClasses;
-    private Set<Pattern> excludedPackageNamePatterns;
-    private Set<String> excludedPackageNames;
-    private Set<String> excludedPackageExemptClasses;
+    private Set<String> excludedClasses = emptySet();
+    private Set<Pattern> excludedPackageNamePatterns = emptySet();
+    private Set<String> excludedPackageNames = emptySet();
+    private Set<String> excludedPackageExemptClasses = emptySet();
 
-    private Set<String> devModeExcludedClasses;
-    private Set<Pattern> devModeExcludedPackageNamePatterns;
-    private Set<String> devModeExcludedPackageNames;
-    private Set<String> devModeExcludedPackageExemptClasses;
+    private Set<String> devModeExcludedClasses = emptySet();
+    private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
+    private Set<String> devModeExcludedPackageNames = emptySet();
+    private Set<String> devModeExcludedPackageExemptClasses = emptySet();
 
     private Container container;
     private boolean allowStaticFieldAccess = true;
@@ -97,48 +102,30 @@ public class OgnlUtil {
     /**
      * Construct a new OgnlUtil instance for use with the framework
      *
-     * @deprecated It is recommended to utilize the {@link OgnlUtil#OgnlUtil(com.opensymphony.xwork2.ognl.ExpressionCacheFactory, com.opensymphony.xwork2.ognl.BeanInfoCacheFactory) method instead.
+     * @deprecated since 6.0.0. Use {@link #OgnlUtil(ExpressionCacheFactory, BeanInfoCacheFactory, OgnlGuard) instead.
      */
     @Deprecated
     public OgnlUtil() {
-        // Instantiate default Expression and BeanInfo caches (factories must be non-null).
-        this(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>());
+        this(new DefaultOgnlExpressionCacheFactory<>(),
+                new DefaultOgnlBeanInfoCacheFactory<>(),
+                new StrutsOgnlGuard());
     }
 
     /**
-     * Construct a new OgnlUtil instance for use with the framework, with optional
-     * cache factories for OGNL Expression and BeanInfo caches.
+     * Construct a new OgnlUtil instance for use with the framework, with optional cache factories for OGNL Expression
+     * and BeanInfo caches.
      *
-     * NOTE: Although the extension points are defined for the optional cache factories, developer-defined overrides do
-     *       do not appear to function at this time (it always appears to instantiate the default factories).
-     *       Construction injectors do not allow the optional flag, so the definitions must be defined.
-     *
-     * @param ognlExpressionCacheFactory factory for Expression cache instance.  If null, it uses a default
-     * @param ognlBeanInfoCacheFactory factory for BeanInfo cache instance.  If null, it uses a default
+     * @param ognlExpressionCacheFactory factory for Expression cache instance
+     * @param ognlBeanInfoCacheFactory   factory for BeanInfo cache instance
+     * @param ognlGuard                  OGNL Guard instance
      */
     @Inject
-    public OgnlUtil(
-            @Inject ExpressionCacheFactory<String, Object> ognlExpressionCacheFactory,
-            @Inject BeanInfoCacheFactory<Class<?>, BeanInfo> ognlBeanInfoCacheFactory
-    ) {
-        if (ognlExpressionCacheFactory == null) {
-            throw new IllegalArgumentException("ExpressionCacheFactory parameter cannot be null");
-        }
-        if (ognlBeanInfoCacheFactory == null) {
-            throw new IllegalArgumentException("BeanInfoCacheFactory parameter cannot be null");
-        }
-        excludedClasses = emptySet();
-        excludedPackageNamePatterns = emptySet();
-        excludedPackageNames = emptySet();
-        excludedPackageExemptClasses = emptySet();
-
-        devModeExcludedClasses = emptySet();
-        devModeExcludedPackageNamePatterns = emptySet();
-        devModeExcludedPackageNames = emptySet();
-        devModeExcludedPackageExemptClasses = emptySet();
-
-        expressionCache = ognlExpressionCacheFactory.buildOgnlCache();
-        beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache();
+    public OgnlUtil(@Inject ExpressionCacheFactory<String, Object> ognlExpressionCacheFactory,
+                    @Inject BeanInfoCacheFactory<Class<?>, BeanInfo> ognlBeanInfoCacheFactory,
+                    @Inject OgnlGuard ognlGuard) {
+        this.expressionCache =  requireNonNull(ognlExpressionCacheFactory).buildOgnlCache();
+        this.beanInfoCache =  requireNonNull(ognlBeanInfoCacheFactory).buildOgnlCache();
+        this.ognlGuard = requireNonNull(ognlGuard);
     }
 
     @Inject
@@ -615,11 +602,14 @@ private Object toTree(String expr) throws OgnlException {
             tree = expressionCache.get(expr);
         }
         if (tree == null) {
-            tree = Ognl.parseExpression(expr);
+            tree = ognlGuard.parseExpression(expr);
             if (enableExpressionCache) {
                 expressionCache.put(expr, tree);
             }
         }
+        if (EXPR_BLOCKED.equals(tree)) {
+            throw new OgnlException("Expression blocked by OgnlGuard: " + expr);
+        }
         return tree;
     }
 

core/src/main/java/org/apache/struts2/StrutsConstants.java

@@ -234,6 +234,8 @@ public final class StrutsConstants {
     /** The name of the parameter to determine whether static field access will be allowed in OGNL expressions or not */
     public static final String STRUTS_ALLOW_STATIC_FIELD_ACCESS = "struts.ognl.allowStaticFieldAccess";
 
+    public static final String STRUTS_OGNL_GUARD = "struts.ognlGuard";
+
     /** The com.opensymphony.xwork2.validator.ActionValidatorManager implementation class */
     public static final String STRUTS_ACTIONVALIDATORMANAGER = "struts.actionValidatorManager";
 
@@ -364,6 +366,9 @@ public final class StrutsConstants {
     /** The maximum length of an expression (OGNL) */
     public static final String STRUTS_OGNL_EXPRESSION_MAX_LENGTH = "struts.ognl.expressionMaxLength";
 
+    /** Parsed OGNL expressions which contain these node types will be blocked */
+    public static final String STRUTS_OGNL_EXCLUDED_NODE_TYPES = "struts.ognl.excludedNodeTypes";
+
     /** Disables {@link org.apache.struts2.dispatcher.StrutsRequestWrapper} request attribute value stack lookup (JSTL accessibility) */
     public static final String STRUTS_DISABLE_REQUEST_ATTRIBUTE_VALUE_STACK_LOOKUP = "struts.disableRequestAttributeValueStackLookup";