This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5339-astmap-block
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1a1318755e8ff1574023754d3fa6388bc8e41498
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Tue Dec 5 13:06:33 2023 +1100
WW-5339 Add option to block custom OGNL maps
---
.../xwork2/ognl/accessor/CompoundRootAccessor.java | 15 +++++++++++-
.../java/org/apache/struts2/StrutsConstants.java | 2 ++
.../com/opensymphony/xwork2/ognl/MyCustomMap.java | 28 ++++++++++++++++++++++
.../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 9 +++++++
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
index 25bedba66..4600c7c97 100644
---
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
+++
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
@@ -42,7 +42,6 @@ import java.beans.PropertyDescriptor;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
-import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
@@ -77,12 +76,18 @@ public class CompoundRootAccessor implements
PropertyAccessor, MethodAccessor, C
private final static Class[] EMPTY_CLASS_ARRAY = new Class[0];
private static final Map<MethodCall, Boolean> invalidMethods = new
ConcurrentHashMap<>();
private boolean devMode;
+ private boolean disallowCustomOgnlMap;
@Inject(StrutsConstants.STRUTS_DEVMODE)
protected void setDevMode(String mode) {
this.devMode = BooleanUtils.toBoolean(mode);
}
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_CUSTOM_OGNL_MAP, required
= false)
+ public void useDisallowCustomOgnlMap(String disallowCustomOgnlMap) {
+ this.disallowCustomOgnlMap =
BooleanUtils.toBoolean(disallowCustomOgnlMap);
+ }
+
public void setProperty(Map context, Object target, Object name, Object
value) throws OgnlException {
CompoundRoot root = (CompoundRoot) target;
OgnlContext ognlContext = (OgnlContext) context;
@@ -275,6 +280,14 @@ public class CompoundRootAccessor implements
PropertyAccessor, MethodAccessor, C
public Class classForName(String className, Map context) throws
ClassNotFoundException {
Object root = Ognl.getRoot(context);
+ if (disallowCustomOgnlMap) {
+ String nodeClassName = ((OgnlContext)
context).getCurrentNode().getClass().getName();
+ if ("ognl.ASTMap".equals(nodeClassName)) {
+ LOG.error("Constructing OGNL ASTMap's from custom classes is
forbidden. Attempted class: {}", className);
+ return null;
+ }
+ }
+
try {
if (root instanceof CompoundRoot) {
if (className.startsWith("vs")) {
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index f5fe67a50..1dc891ee2 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_DISALLOW_CUSTOM_OGNL_MAP =
"struts.ognl.disallowCustomOgnlMap";
+
public static final String STRUTS_MEMBER_ACCESS =
"struts.securityMemberAccess";
public static final String STRUTS_OGNL_GUARD = "struts.ognlGuard";
diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/MyCustomMap.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/MyCustomMap.java
new file mode 100644
index 000000000..ef5683386
--- /dev/null
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/MyCustomMap.java
@@ -0,0 +1,28 @@
+/*
+ * 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 java.util.HashMap;
+
+public class MyCustomMap<K, V> extends HashMap<K, V> {
+ @Override
+ public V get(Object key) {
+ return (V) "System compromised";
+ }
+}
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 5df410e2b..fe94e7e70 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -26,6 +26,7 @@ import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.inject.ContainerBuilder;
import com.opensymphony.xwork2.interceptor.ChainingInterceptor;
+import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.test.StubConfigurationProvider;
import com.opensymphony.xwork2.test.User;
import com.opensymphony.xwork2.util.Bar;
@@ -35,6 +36,7 @@ import com.opensymphony.xwork2.util.Owner;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
+import ognl.ClassResolver;
import ognl.InappropriateExpressionException;
import ognl.MethodFailedException;
import ognl.NoSuchPropertyException;
@@ -1623,6 +1625,13 @@ public class OgnlUtilTest extends XWorkTestCase {
assertEquals("Eviction limit for cache mismatches limit for factory
?", 15, ognlCache.getEvictionLimit());
}
+ public void testCustomOgnlMapBlocked() throws Exception {
+ assertEquals("System compromised",
ognlUtil.getValue("#@com.opensymphony.xwork2.ognl.MyCustomMap@{}.get(\"ye\")",
ognlUtil.createDefaultContext(null), null));
+ ((CompoundRootAccessor) container.getInstance(ClassResolver.class,
CompoundRoot.class.getName()))
+ .useDisallowCustomOgnlMap(Boolean.TRUE.toString());
+ assertThrows(OgnlException.class, () ->
ognlUtil.getValue("#@com.opensymphony.xwork2.ognl.MyCustomMap@{}.get(\"ye\")",
ognlUtil.createDefaultContext(null), null));
+ }
+
/**
* Generate a new OgnlUtil instance (not configured by the {@link
ContainerBuilder}) that can be used for
* basic tests, with its Expression and BeanInfo factories set to LRU mode.
