This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5627-cookie-authorization in repository https://gitbox.apache.org/repos/asf/struts.git
commit 17c076e2a269704aff7e38a3b55860d8e9ae8558 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat May 9 17:10:33 2026 +0200 WW-5627 add OgnlParameterAllowlister default implementation Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../parameter/OgnlParameterAllowlister.java | 163 +++++++++++++++++++++ .../parameter/OgnlParameterAllowlisterTest.java | 156 ++++++++++++++++++++ 2 files changed, 319 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java new file mode 100644 index 000000000..c38015cfa --- /dev/null +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java @@ -0,0 +1,163 @@ +/* + * 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.interceptor.parameter; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.OgnlUtil; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.util.ProxyService; + +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.PropertyDescriptor; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Arrays; +import java.util.Optional; + +import static org.apache.commons.lang3.StringUtils.indexOfAny; +import static org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS; +import static org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS_STR; + +/** + * Default {@link ParameterAllowlister} that primes OGNL {@link ThreadAllowlist} for nested-path writes. Logic is + * extracted verbatim from {@code ParametersInterceptor.performOgnlAllowlisting} so the OGNL parameter and cookie + * channels share a single implementation. + * + * <p>No-ops for depth-0 paths — root-level setters do not need allowlisting.</p> + * + * @since 7.2.0 + */ +public class OgnlParameterAllowlister implements ParameterAllowlister { + + private static final Logger LOG = LogManager.getLogger(OgnlParameterAllowlister.class); + + private OgnlUtil ognlUtil; + private ProxyService proxyService; + private ThreadAllowlist threadAllowlist; + + @Inject + public void setOgnlUtil(OgnlUtil ognlUtil) { + this.ognlUtil = ognlUtil; + } + + @Inject + public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; + } + + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + + @Override + public void allowlistAuthorizedPath(String parameterName, Object target) { + if (parameterName == null || parameterName.isEmpty() || target == null) { + return; + } + long paramDepth = parameterName.codePoints().mapToObj(c -> (char) c).filter(NESTING_CHARS::contains).count(); + if (paramDepth == 0) { + return; + } + + int nestingIndex = indexOfAny(parameterName, NESTING_CHARS_STR); + String rootProperty = nestingIndex == -1 ? parameterName : parameterName.substring(0, nestingIndex); + String normalisedRootProperty = Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1); + + BeanInfo beanInfo = getBeanInfo(target); + if (beanInfo != null) { + Optional<PropertyDescriptor> propDescOpt = Arrays.stream(beanInfo.getPropertyDescriptors()) + .filter(desc -> desc.getName().equals(normalisedRootProperty)).findFirst(); + if (propDescOpt.isPresent()) { + PropertyDescriptor propDesc = propDescOpt.get(); + Method relevantMethod = propDesc.getReadMethod(); + if (relevantMethod != null && getPermittedInjectionDepth(relevantMethod) >= paramDepth) { + allowlistClass(propDesc.getPropertyType()); + if (paramDepth >= 2) { + allowlistParameterizedTypeArg(relevantMethod.getGenericReturnType()); + } + return; + } + } + } + + Class<?> targetClass = ultimateClass(target); + try { + Field field = targetClass.getDeclaredField(normalisedRootProperty); + if (Modifier.isPublic(field.getModifiers()) && getPermittedInjectionDepth(field) >= paramDepth) { + allowlistClass(field.getType()); + if (paramDepth >= 2) { + allowlistParameterizedTypeArg(field.getGenericType()); + } + } + } catch (NoSuchFieldException e) { + // No public field by that name — nothing to allowlist. + } + } + + private void allowlistClass(Class<?> clazz) { + threadAllowlist.allowClassHierarchy(clazz); + } + + private void allowlistParameterizedTypeArg(Type genericType) { + if (!(genericType instanceof ParameterizedType pType)) { + return; + } + Type[] paramTypes = pType.getActualTypeArguments(); + allowlistParamType(paramTypes[0]); + if (paramTypes.length > 1) { + allowlistParamType(paramTypes[1]); + } + } + + private void allowlistParamType(Type paramType) { + if (paramType instanceof Class<?> clazz) { + allowlistClass(clazz); + } + } + + private int getPermittedInjectionDepth(AnnotatedElement element) { + StrutsParameter annotation = element.getAnnotation(StrutsParameter.class); + return annotation == null ? -1 : annotation.depth(); + } + + private Class<?> ultimateClass(Object target) { + if (proxyService.isProxy(target)) { + return proxyService.ultimateTargetClass(target); + } + return target.getClass(); + } + + private BeanInfo getBeanInfo(Object target) { + Class<?> targetClass = ultimateClass(target); + try { + return ognlUtil.getBeanInfo(targetClass); + } catch (IntrospectionException e) { + LOG.warn("Error introspecting target {} for OGNL allowlisting", targetClass, e); + return null; + } + } +} diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java new file mode 100644 index 000000000..0376da80b --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java @@ -0,0 +1,156 @@ +/* + * 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.interceptor.parameter; + +import org.apache.struts2.ognl.DefaultOgnlBeanInfoCacheFactory; +import org.apache.struts2.ognl.DefaultOgnlExpressionCacheFactory; +import org.apache.struts2.ognl.OgnlUtil; +import org.apache.struts2.ognl.StrutsOgnlGuard; +import org.apache.struts2.ognl.StrutsProxyCacheFactory; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.util.StrutsProxyService; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.apache.struts2.ognl.OgnlCacheFactory.CacheType.LRU; +import static org.assertj.core.api.Assertions.assertThat; + +public class OgnlParameterAllowlisterTest { + + private OgnlParameterAllowlister allowlister; + private RecordingThreadAllowlist threadAllowlist; + + @Before + public void setUp() { + threadAllowlist = new RecordingThreadAllowlist(); + allowlister = new OgnlParameterAllowlister(); + var ognlUtil = new OgnlUtil( + new DefaultOgnlExpressionCacheFactory<>(String.valueOf(1000), LRU.toString()), + new DefaultOgnlBeanInfoCacheFactory<>(String.valueOf(1000), LRU.toString()), + new StrutsOgnlGuard()); + allowlister.setOgnlUtil(ognlUtil); + allowlister.setProxyService(new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic"))); + allowlister.setThreadAllowlist(threadAllowlist); + } + + @After + public void tearDown() { + threadAllowlist.clear(); + } + + @Test + public void depthZero_isNoOp() { + var target = new TargetWithAnnotatedNestedBean(); + allowlister.allowlistAuthorizedPath("simple", target); + assertThat(threadAllowlist.classes).isEmpty(); + } + + @Test + public void nestedProperty_allowlistsPropertyType() { + var target = new TargetWithAnnotatedNestedBean(); + allowlister.allowlistAuthorizedPath("nested.field", target); + assertThat(threadAllowlist.classes).contains(NestedBean.class); + } + + @Test + public void parameterizedReturn_allowlistsTypeArguments() { + var target = new TargetWithAnnotatedNestedBean(); + allowlister.allowlistAuthorizedPath("things[0].field", target); + assertThat(threadAllowlist.classes).contains(List.class, NestedBean.class); + } + + @Test + public void publicField_isAllowlistedWhenNoGetter() { + var target = new TargetWithAnnotatedPublicField(); + allowlister.allowlistAuthorizedPath("publicNested.field", target); + assertThat(threadAllowlist.classes).contains(NestedBean.class); + } + + @Test + public void unmatchedRoot_isNoOp() { + var target = new TargetWithAnnotatedNestedBean(); + allowlister.allowlistAuthorizedPath("unknownRoot.field", target); + assertThat(threadAllowlist.classes).isEmpty(); + } + + @Test + public void unannotatedNested_isNoOp() { + var target = new TargetWithUnannotatedNested(); + allowlister.allowlistAuthorizedPath("unannotated.field", target); + assertThat(threadAllowlist.classes).isEmpty(); + } + + public static class TargetWithAnnotatedNestedBean { + private NestedBean nested; + private List<NestedBean> things; + private Map<String, NestedBean> mapping; + + @StrutsParameter(depth = 1) + public NestedBean getNested() { return nested; } + public void setNested(NestedBean nested) { this.nested = nested; } + + @StrutsParameter(depth = 2) + public List<NestedBean> getThings() { return things; } + public void setThings(List<NestedBean> things) { this.things = things; } + } + + public static class TargetWithAnnotatedPublicField { + @StrutsParameter(depth = 1) + public NestedBean publicNested; + } + + public static class TargetWithUnannotatedNested { + private NestedBean unannotated; + public NestedBean getUnannotated() { return unannotated; } + public void setUnannotated(NestedBean v) { this.unannotated = v; } + } + + public static class NestedBean { + private String field; + public String getField() { return field; } + public void setField(String f) { this.field = f; } + } + + private static final class RecordingThreadAllowlist extends ThreadAllowlist { + final Set<Class<?>> classes = new HashSet<>(); + + @Override + public void allowClass(Class<?> clazz) { + classes.add(clazz); + super.allowClass(clazz); + } + + @Override + public void allowClassHierarchy(Class<?> clazz) { + classes.add(clazz); + super.allowClassHierarchy(clazz); + } + + void clear() { + classes.clear(); + clearAllowlist(); + } + } +}
