You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ku...@apache.org on 2023/08/31 11:35:07 UTC
[struts] 01/04: WW-5340 Introducing OGNL Guard
This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5340-ognl-guard
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 7a9c61cb69a8ad1f7b6e07fdeb278ee704119ca2
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Thu Aug 31 19:21:51 2023 +1000
WW-5340 Introducing OGNL Guard
---
.../xwork2/config/impl/DefaultConfiguration.java | 85 +++++++++++++++++++---
.../StrutsDefaultConfigurationProvider.java | 3 +
.../opensymphony/xwork2/ognl/DefaultOgnlGuard.java | 79 ++++++++++++++++++++
.../com/opensymphony/xwork2/ognl/OgnlGuard.java | 37 ++++++++++
.../com/opensymphony/xwork2/ognl/OgnlUtil.java | 42 +++++------
.../java/org/apache/struts2/StrutsConstants.java | 3 +
core/src/main/resources/struts-beans.xml | 1 +
7 files changed, 217 insertions(+), 33 deletions(-)
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 71fdf2ff8..92852657e 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,25 +18,82 @@
*/
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;
+import com.opensymphony.xwork2.ognl.DefaultOgnlGuard;
import com.opensymphony.xwork2.ognl.ExpressionCacheFactory;
+import com.opensymphony.xwork2.ognl.OgnlGuard;
import com.opensymphony.xwork2.ognl.OgnlReflectionProvider;
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 +104,17 @@ import org.apache.logging.log4j.LogManager;
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 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 @@ public class DefaultConfiguration implements Configuration {
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, DefaultOgnlGuard.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 2c304ec82..2f9da8892 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
@@ -68,8 +68,10 @@ import com.opensymphony.xwork2.inject.Scope;
import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory;
import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory;
import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory;
+import com.opensymphony.xwork2.ognl.DefaultOgnlGuard;
import com.opensymphony.xwork2.ognl.ExpressionCacheFactory;
import com.opensymphony.xwork2.ognl.ObjectProxy;
+import com.opensymphony.xwork2.ognl.OgnlGuard;
import com.opensymphony.xwork2.ognl.OgnlReflectionContextFactory;
import com.opensymphony.xwork2.ognl.OgnlReflectionProvider;
import com.opensymphony.xwork2.ognl.OgnlUtil;
@@ -227,6 +229,7 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider
.factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON)
.factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON)
.factory(OgnlUtil.class, Scope.SINGLETON)
+ .factory(OgnlGuard.class, DefaultOgnlGuard.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/DefaultOgnlGuard.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.java
new file mode 100644
index 000000000..0d70d515c
--- /dev/null
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.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 com.opensymphony.xwork2.ognl;
+
+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 DefaultOgnlGuard implements OgnlGuard {
+
+ private static final Logger LOG = LogManager.getLogger(DefaultOgnlGuard.class);
+
+ private Set<String> excludedNodeTypes = emptySet();
+
+ @Inject(value = StrutsConstants.STRUTS_OGNL_EXCLUDED_NODE_TYPES, required = false)
+ public void useExcludedNodeTypes(String excludedNodeTypes) {
+ Set<String> incomingExcludedNodeTypes = commaDelimitedStringToSet(excludedNodeTypes);
+ Set<String> newExcludeNodeTypes = new HashSet<>(this.excludedNodeTypes);
+ newExcludeNodeTypes.addAll(incomingExcludedNodeTypes);
+ this.excludedNodeTypes = unmodifiableSet(newExcludeNodeTypes);
+ }
+
+ @Override
+ public boolean isBlocked(String expr, 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 (containsExcludedNodeType(node.jjtGetChild(i))) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+}
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
new file mode 100644
index 000000000..b1bcb409e
--- /dev/null
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
@@ -0,0 +1,37 @@
+/*
+ * 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 com.opensymphony.xwork2.ognl;
+
+/**
+ * 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 {
+
+ /**
+ * It is imperative that the parsed tree matches the expression.
+ *
+ * @param expr OGNL expression
+ * @param tree parsed tree of expression
+ * @return whether the expression should be blocked
+ */
+ boolean isBlocked(String expr, Object tree);
+}
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 8edac0a95..c080500aa 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -53,6 +53,7 @@ import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import static com.opensymphony.xwork2.util.TextParseUtil.commaDelimitedStringToSet;
+import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toSet;
import static org.apache.commons.lang3.StringUtils.strip;
@@ -73,6 +74,7 @@ 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;
@@ -95,37 +97,27 @@ 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<>());
+ new DefaultOgnlBeanInfoCacheFactory<>(),
+ new DefaultOgnlGuard());
}
/**
- * 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");
- }
+ public OgnlUtil(@Inject ExpressionCacheFactory<String, Object> ognlExpressionCacheFactory,
+ @Inject BeanInfoCacheFactory<Class<?>, BeanInfo> ognlBeanInfoCacheFactory,
+ @Inject OgnlGuard ognlGuard) {
excludedClasses = Collections.unmodifiableSet(new HashSet<>());
excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>());
excludedPackageNames = Collections.unmodifiableSet(new HashSet<>());
@@ -136,8 +128,9 @@ public class OgnlUtil {
devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>());
devModeExcludedPackageExemptClasses = Collections.unmodifiableSet(new HashSet<>());
- this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache();
- this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache();
+ this.expressionCache = requireNonNull(ognlExpressionCacheFactory).buildOgnlCache();
+ this.beanInfoCache = requireNonNull(ognlBeanInfoCacheFactory).buildOgnlCache();
+ this.ognlGuard = requireNonNull(ognlGuard);
}
@Inject
@@ -624,6 +617,9 @@ public class OgnlUtil {
expressionCache.put(expr, tree);
}
}
+ if (ognlGuard.isBlocked(expr, 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 29dfca4b9..143b8c832 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -364,6 +364,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/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml
index fc1fb2ee7..89acda7c6 100644
--- a/core/src/main/resources/struts-beans.xml
+++ b/core/src/main/resources/struts-beans.xml
@@ -166,6 +166,7 @@
class="com.opensymphony.xwork2.validator.DefaultValidatorFileParser"/>
<bean class="com.opensymphony.xwork2.ognl.OgnlUtil"/>
+ <bean type="com.opensymphony.xwork2.ognl.OgnlGuard" class="com.opensymphony.xwork2.ognl.DefaultOgnlGuard"/>
<bean type="com.opensymphony.xwork2.util.TextParser" name="struts"
class="com.opensymphony.xwork2.util.OgnlTextParser" scope="singleton"/>