diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 71fdf2ff8c..3deda2c812 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/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 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); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index 2c304ec829..5769228569 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/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) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java index 83893c1539..fc83666991 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java +++ b/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 The type for the cache key entries * @param The type for the cache value entries */ public interface OgnlCache { - 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); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java index 20431e133e..a32736da67 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java +++ b/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 The type for the cache key entries * @param The type for the cache value entries */ diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index a99adca2a5..93ab56d363 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/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 The type for the cache key entries * @param The type for the cache value entries */ diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 4f691543a2..4bddad63ae 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/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 expressionCache; private final OgnlCache, BeanInfo> beanInfoCache; private TypeConverter defaultConverter; + private final OgnlGuard ognlGuard; private boolean devMode; private boolean enableExpressionCache = true; private boolean enableEvalExpression; - private Set excludedClasses; - private Set excludedPackageNamePatterns; - private Set excludedPackageNames; - private Set excludedPackageExemptClasses; + private Set excludedClasses = emptySet(); + private Set excludedPackageNamePatterns = emptySet(); + private Set excludedPackageNames = emptySet(); + private Set excludedPackageExemptClasses = emptySet(); - private Set devModeExcludedClasses; - private Set devModeExcludedPackageNamePatterns; - private Set devModeExcludedPackageNames; - private Set devModeExcludedPackageExemptClasses; + private Set devModeExcludedClasses = emptySet(); + private Set devModeExcludedPackageNamePatterns = emptySet(); + private Set devModeExcludedPackageNames = emptySet(); + private Set 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 ognlExpressionCacheFactory, - @Inject BeanInfoCacheFactory, 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 ognlExpressionCacheFactory, + @Inject BeanInfoCacheFactory, 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; } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index f93370eaa6..b965a0614a 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/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"; diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index c80494f356..70cc85135c 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -19,18 +19,13 @@ package org.apache.struts2.config; import com.opensymphony.xwork2.ActionProxyFactory; -import com.opensymphony.xwork2.LocaleProviderFactory; -import com.opensymphony.xwork2.LocalizedTextProvider; -import com.opensymphony.xwork2.TextProviderFactory; -import com.opensymphony.xwork2.factory.UnknownHandlerFactory; -import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory; -import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; -import com.opensymphony.xwork2.security.AcceptedPatternsChecker; -import com.opensymphony.xwork2.security.ExcludedPatternsChecker; 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.TextProvider; +import com.opensymphony.xwork2.TextProviderFactory; import com.opensymphony.xwork2.UnknownHandlerManager; import com.opensymphony.xwork2.conversion.ConversionAnnotationProcessor; import com.opensymphony.xwork2.conversion.ConversionFileProcessor; @@ -48,9 +43,14 @@ import com.opensymphony.xwork2.factory.ConverterFactory; import com.opensymphony.xwork2.factory.InterceptorFactory; import com.opensymphony.xwork2.factory.ResultFactory; +import com.opensymphony.xwork2.factory.UnknownHandlerFactory; import com.opensymphony.xwork2.factory.ValidatorFactory; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.inject.Scope; +import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory; +import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.PatternMatcher; import com.opensymphony.xwork2.util.TextParser; @@ -67,6 +67,7 @@ import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; import org.apache.struts2.interceptor.exec.ExecutorProvider; +import org.apache.struts2.ognl.OgnlGuard; import org.apache.struts2.url.QueryStringBuilder; import org.apache.struts2.url.QueryStringParser; import org.apache.struts2.url.UrlDecoder; @@ -434,6 +435,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) { alias(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, builder, props, Scope.SINGLETON); alias(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(OgnlGuard.class, StrutsConstants.STRUTS_OGNL_GUARD, builder, props, Scope.SINGLETON); + alias(QueryStringBuilder.class, StrutsConstants.STRUTS_URL_QUERY_STRING_BUILDER, builder, props, Scope.SINGLETON); alias(QueryStringParser.class, StrutsConstants.STRUTS_URL_QUERY_STRING_PARSER, builder, props, Scope.SINGLETON); alias(UrlEncoder.class, StrutsConstants.STRUTS_URL_ENCODER, builder, props, Scope.SINGLETON); diff --git a/core/src/main/java/org/apache/struts2/ognl/OgnlGuard.java b/core/src/main/java/org/apache/struts2/ognl/OgnlGuard.java new file mode 100644 index 0000000000..8924c4684c --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/OgnlGuard.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.ognl; + +import com.opensymphony.xwork2.ognl.OgnlUtil; +import ognl.Ognl; +import ognl.OgnlException; + +/** + * Guards all expressions parsed by Struts Core. It is evaluated by {@link OgnlUtil} immediately after parsing any + * expression. + * + * @since 6.4.0 + */ +public interface OgnlGuard { + + String EXPR_BLOCKED = "_ognl_guard_blocked"; + + /** + * Determines whether an OGNL expression should be blocked based on validation done on both the raw expression and + * the parsed tree. + * + * @param expr OGNL expression + * @return whether the expression should be blocked + */ + default boolean isBlocked(String expr) throws OgnlException { + return EXPR_BLOCKED.equals(parseExpression(expr)); + } + + /** + * Parses an OGNL expression and returns the resulting tree only if the expression is not blocked as per defined + * validation rules in {@link #isRawExpressionBlocked} and {@link #isParsedTreeBlocked}. + * + * @param expr OGNL expression + * @return parsed expression or {@link #EXPR_BLOCKED} if the expression should be blocked + */ + default Object parseExpression(String expr) throws OgnlException { + if (isRawExpressionBlocked(expr)) { + return EXPR_BLOCKED; + } + Object tree = Ognl.parseExpression(expr); + if (isParsedTreeBlocked(tree)) { + return EXPR_BLOCKED; + } + return tree; + } + + /** + * Determines whether an OGNL expression should be blocked based on validation done on only the raw expression, + * without parsing the tree. + * + * @param expr OGNL expression + * @return whether the expression should be blocked + */ + boolean isRawExpressionBlocked(String expr); + + /** + * Determines whether a parsed OGNL tree should be blocked based on some validation rules. + * + * @param tree parsed OGNL tree + * @return whether the parsed tree should be blocked + */ + boolean isParsedTreeBlocked(Object tree); +} diff --git a/core/src/main/java/org/apache/struts2/ognl/StrutsOgnlGuard.java b/core/src/main/java/org/apache/struts2/ognl/StrutsOgnlGuard.java new file mode 100644 index 0000000000..262aec3626 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/StrutsOgnlGuard.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.ognl; + +import com.opensymphony.xwork2.config.ConfigurationException; +import com.opensymphony.xwork2.inject.Inject; +import ognl.Node; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.StrutsConstants; + +import java.util.HashSet; +import java.util.Set; + +import static com.opensymphony.xwork2.util.TextParseUtil.commaDelimitedStringToSet; +import static java.util.Collections.emptySet; +import static java.util.Collections.unmodifiableSet; + +/** + * The default implementation of {@link OgnlGuard}. + * + * @since 6.4.0 + */ +public class StrutsOgnlGuard implements OgnlGuard { + + private static final Logger LOG = LogManager.getLogger(StrutsOgnlGuard.class); + + protected Set excludedNodeTypes = emptySet(); + + @Inject(value = StrutsConstants.STRUTS_OGNL_EXCLUDED_NODE_TYPES, required = false) + public void useExcludedNodeTypes(String excludedNodeTypes) { + Set incomingExcludedNodeTypes = commaDelimitedStringToSet(excludedNodeTypes); + validateExcludedNodeTypes(incomingExcludedNodeTypes); + Set newExcludeNodeTypes = new HashSet<>(this.excludedNodeTypes); + newExcludeNodeTypes.addAll(incomingExcludedNodeTypes); + this.excludedNodeTypes = unmodifiableSet(newExcludeNodeTypes); + } + + protected void validateExcludedNodeTypes(Set incomingExcludedNodeTypes) throws ConfigurationException { + for (String excludedNodeType : incomingExcludedNodeTypes) { + try { + if (!Node.class.isAssignableFrom(Class.forName(excludedNodeType))) { + throw new ConfigurationException("Excluded node type [" + excludedNodeType + "] is not a subclass of " + Node.class.getName()); + } + } catch (ClassNotFoundException e) { + throw new ConfigurationException("Excluded node type [" + excludedNodeType + "] does not exist or cannot be loaded"); + } + } + } + + @Override + public boolean isRawExpressionBlocked(String expr) { + return false; + } + + @Override + public boolean isParsedTreeBlocked(Object tree) { + return containsExcludedNodeType(tree); + } + + protected boolean containsExcludedNodeType(Object tree) { + if (!(tree instanceof Node) || excludedNodeTypes.isEmpty()) { + return false; + } + return recurseExcludedNodeType((Node) tree); + } + + protected boolean recurseExcludedNodeType(Node node) { + String nodeClassName = node.getClass().getName(); + if (excludedNodeTypes.contains(nodeClassName)) { + LOG.warn("Expression contains blocked node type [{}]", nodeClassName); + return true; + } else { + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + if (recurseExcludedNodeType(node.jjtGetChild(i))) { + return true; + } + } + return false; + } + } +} diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index fc1fb2ee7c..3dcd60197b 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -166,6 +166,8 @@ class="com.opensymphony.xwork2.validator.DefaultValidatorFileParser"/> + diff --git a/core/src/main/resources/struts-excluded-classes.xml b/core/src/main/resources/struts-excluded-classes.xml index 72df88c898..226f064bbf 100644 --- a/core/src/main/resources/struts-excluded-classes.xml +++ b/core/src/main/resources/struts-excluded-classes.xml @@ -77,6 +77,7 @@ com.opensymphony.xwork2.ognl, com.opensymphony.xwork2.security, com.opensymphony.xwork2.util, + org.apache.struts2.ognl, org.apache.tomcat, org.apache.catalina.core, org.wildfly.extension.undertow.deployment"/> diff --git a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java index db6e21b643..e099a060a6 100644 --- a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java @@ -33,6 +33,7 @@ import com.opensymphony.xwork2.util.ValueStackFactory; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.ognl.StrutsOgnlGuard; import java.util.ArrayList; import java.util.HashMap; @@ -532,8 +533,9 @@ protected void setUp() throws Exception { private OgnlUtil createOgnlUtil() { return new OgnlUtil( - new DefaultOgnlExpressionCacheFactory<>(), - new DefaultOgnlBeanInfoCacheFactory<>() + new DefaultOgnlExpressionCacheFactory<>(), + new DefaultOgnlBeanInfoCacheFactory<>(), + new StrutsOgnlGuard() ); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 1222fdff4f..eb8aef83cc 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -46,6 +46,7 @@ import ognl.SimpleNode; import org.apache.struts2.StrutsConstants; import org.apache.struts2.StrutsException; +import org.apache.struts2.ognl.StrutsOgnlGuard; import java.beans.BeanInfo; import java.beans.IntrospectionException; @@ -1347,22 +1348,23 @@ public void testDefaultOgnlUtilExclusions() { public void testDefaultOgnlUtilAlternateConstructorArguments() { // Code coverage test for the OgnlUtil alternate constructor method, and verify expected behaviour. try { - OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), null); + OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), null, null); fail("null beanInfoCacheFactory should result in exception"); - } catch (IllegalArgumentException iaex) { + } catch (NullPointerException iaex) { // expected result } try { - OgnlUtil basicOgnlUtil = new OgnlUtil(null, new DefaultOgnlBeanInfoCacheFactory<>()); + OgnlUtil basicOgnlUtil = new OgnlUtil(null, new DefaultOgnlBeanInfoCacheFactory<>(), null); fail("null expressionCacheFactory should result in exception"); - } catch (IllegalArgumentException iaex) { + } catch (NullPointerException iaex) { // expected result } } public void testDefaultOgnlUtilExclusionsAlternateConstructorPopulated() { OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), - new DefaultOgnlBeanInfoCacheFactory<>()); + new DefaultOgnlBeanInfoCacheFactory<>(), + new StrutsOgnlGuard()); internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil); internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); @@ -1709,7 +1711,9 @@ public void testGetExcludedPackageNames() { public void testGetExcludedPackageNamesAlternateConstructorPopulated() { // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>()); + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), + new DefaultOgnlBeanInfoCacheFactory<>(), + new StrutsOgnlGuard()); util.setExcludedPackageNames("java.lang,java.awt"); assertEquals(util.getExcludedPackageNames().size(), 2); try { @@ -1737,7 +1741,9 @@ public void testGetExcludedClasses() { public void testGetExcludedClassesAlternateConstructorPopulated() { // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>()); + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), + new DefaultOgnlBeanInfoCacheFactory<>(), + new StrutsOgnlGuard()); util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL"); assertEquals(util.getExcludedClasses().size(), 3); try { @@ -1769,7 +1775,9 @@ public void testInvalidExcludedPackageNamePatterns() { public void testGetExcludedPackageNamePatternsAlternateConstructorPopulated() { // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>()); + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), + new DefaultOgnlBeanInfoCacheFactory<>(), + new StrutsOgnlGuard()); util.setExcludedPackageNamePatterns("java.lang."); assertEquals(util.getExcludedPackageNamePatterns().size(), 1); try { @@ -1875,7 +1883,7 @@ private OgnlUtil generateOgnlUtilInstanceWithDefaultLRUCacheFactories() { expressionFactory.setCacheMaxSize("25"); beanInfoFactory.setUseLRUCache("true"); beanInfoFactory.setCacheMaxSize("25"); - result = new OgnlUtil(expressionFactory, beanInfoFactory); + result = new OgnlUtil(expressionFactory, beanInfoFactory, new StrutsOgnlGuard()); return result; } diff --git a/core/src/test/java/org/apache/struts2/ognl/StrutsOgnlGuardTest.java b/core/src/test/java/org/apache/struts2/ognl/StrutsOgnlGuardTest.java new file mode 100644 index 0000000000..2252134da2 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/ognl/StrutsOgnlGuardTest.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.ognl; + +import com.opensymphony.xwork2.config.ConfigurationException; +import org.junit.Before; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class StrutsOgnlGuardTest { + + private StrutsOgnlGuard strutsOgnlGuard; + + @Before + public void setUp() throws Exception { + strutsOgnlGuard = new StrutsOgnlGuard(); + } + + @Test + public void notConfigured() throws Exception { + String expr = "1+1"; + assertFalse(strutsOgnlGuard.isBlocked(expr)); + } + + @Test + public void nonNodeTree() throws Exception { + assertFalse(strutsOgnlGuard.isParsedTreeBlocked("String")); + } + + @Test + public void nonExistentClass() throws Exception { + ConfigurationException e = assertThrows(ConfigurationException.class, () -> strutsOgnlGuard.useExcludedNodeTypes("ognl.Kusal")); + assertThat(e).hasMessageContaining("does not exist or cannot be loaded"); + } + + @Test + public void invalidClass() throws Exception { + ConfigurationException e = assertThrows(ConfigurationException.class, () -> strutsOgnlGuard.useExcludedNodeTypes("ognl.ElementsAccessor")); + assertThat(e).hasMessageContaining("is not a subclass of ognl.Node"); + } + + @Test + public void addBlocked() throws Exception { + String expr = "1+1"; + assertFalse(strutsOgnlGuard.isBlocked(expr)); + + strutsOgnlGuard.useExcludedNodeTypes("ognl.ASTAdd"); + assertTrue(strutsOgnlGuard.isBlocked(expr)); + } + + @Test + public void nestedAddBlocked() throws Exception { + String expr = "{'a',1+1}"; + assertFalse(strutsOgnlGuard.isBlocked(expr)); + + strutsOgnlGuard.useExcludedNodeTypes("ognl.ASTAdd"); + assertTrue(strutsOgnlGuard.isBlocked(expr)); + } +}