WW-5339 Add option to block custom OGNL maps

This commit is contained in:
Kusal Kithul-Godage
2023-12-05 13:06:33 +11:00
parent 48a82feada
commit 002e598b75
4 changed files with 56 additions and 1 deletions
@@ -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")) {
@@ -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";
@@ -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";
}
}
@@ -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,16 @@ public class OgnlUtilTest extends XWorkTestCase {
assertEquals("Eviction limit for cache mismatches limit for factory ?", 15, ognlCache.getEvictionLimit());
}
public void testCustomOgnlMapBlocked() throws Exception {
String vulnerableExpr = "#@com.opensymphony.xwork2.ognl.MyCustomMap@{}.get(\"ye\")";
assertEquals("System compromised", ognlUtil.getValue(vulnerableExpr, ognlUtil.createDefaultContext(null), null));
((CompoundRootAccessor) container.getInstance(ClassResolver.class, CompoundRoot.class.getName()))
.useDisallowCustomOgnlMap(Boolean.TRUE.toString());
assertThrows(OgnlException.class, () -> ognlUtil.getValue(vulnerableExpr, 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.