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 681aac57d7..78cada96df 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -47,7 +47,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Pattern; import static com.opensymphony.xwork2.util.ConfigParseUtil.toClassesSet; @@ -68,9 +67,6 @@ public class OgnlUtil { private static final Logger LOG = LogManager.getLogger(OgnlUtil.class); - // Flag used to reduce flooding logs with WARNs about using DevMode excluded packages - private final AtomicBoolean warnReported = new AtomicBoolean(false); - private final OgnlCache expressionCache; private final OgnlCache, BeanInfo> beanInfoCache; private TypeConverter defaultConverter; @@ -80,11 +76,6 @@ public class OgnlUtil { private boolean enableExpressionCache = true; private boolean enableEvalExpression; - private String devModeExcludedClasses = ""; - private String devModeExcludedPackageNamePatterns = ""; - private String devModeExcludedPackageNames = ""; - private String devModeExcludedPackageExemptClasses = ""; - private Container container; /** @@ -164,9 +155,12 @@ protected void setExcludedClasses(String commaDelimitedClasses) { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedClasses(String commaDelimitedClasses) { - this.devModeExcludedClasses = commaDelimitedClasses; + // Must be set directly on SecurityMemberAccess } /** @@ -177,9 +171,12 @@ protected void setExcludedPackageNamePatterns(String commaDelimitedPackagePatter // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { - this.devModeExcludedPackageNamePatterns = commaDelimitedPackagePatterns; + // Must be set directly on SecurityMemberAccess } /** @@ -190,9 +187,12 @@ protected void setExcludedPackageNames(String commaDelimitedPackageNames) { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedPackageNames(String commaDelimitedPackageNames) { - this.devModeExcludedPackageNames = commaDelimitedPackageNames; + // Must be set directly on SecurityMemberAccess } /** @@ -203,9 +203,12 @@ public void setExcludedPackageExemptClasses(String commaDelimitedClasses) { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated public void setDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) { - this.devModeExcludedPackageExemptClasses = commaDelimitedClasses; + // Must be set directly on SecurityMemberAccess } /** @@ -856,6 +859,11 @@ protected Map createDefaultContext(Object root) { return createDefaultContext(root, null); } + /** + * Note that the allowlist capability is not enforced by the {@link OgnlContext} returned by this method. Currently, + * this context is only leveraged by some public methods on {@link OgnlUtil} which are called by + * {@link OgnlReflectionProvider}. + */ protected Map createDefaultContext(Object root, ClassResolver resolver) { if (resolver == null) { resolver = container.getInstance(RootAccessor.class); @@ -867,17 +875,6 @@ protected Map createDefaultContext(Object root, ClassResolver re SecurityMemberAccess memberAccess = container.getInstance(SecurityMemberAccess.class); memberAccess.useEnforceAllowlistEnabled(Boolean.FALSE.toString()); - if (devMode) { - if (!warnReported.get()) { - warnReported.set(true); - LOG.warn("Working in devMode, using devMode excluded classes and packages!"); - } - memberAccess.useExcludedClasses(devModeExcludedClasses); - memberAccess.useExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns); - memberAccess.useExcludedPackageNames(devModeExcludedPackageNames); - memberAccess.useExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses); - } - return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 0c16ca1a6e..f225b3c89c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -90,7 +90,12 @@ public class SecurityMemberAccess implements MemberAccess { private Set excludedPackageNames = emptySet(); private Set excludedPackageExemptClasses = emptySet(); + private volatile boolean isDevModeInit; private boolean isDevMode; + private Set devModeExcludedClasses = unmodifiableSet(new HashSet<>(singletonList(Object.class.getName()))); + private Set devModeExcludedPackageNamePatterns = emptySet(); + private Set devModeExcludedPackageNames = emptySet(); + private Set devModeExcludedPackageExemptClasses = emptySet(); private boolean enforceAllowlistEnabled = false; private Set> allowlistClasses = emptySet(); @@ -296,6 +301,7 @@ protected boolean isClassAllowlisted(Class clazz) { * @return {@code true} if member access is allowed */ protected boolean checkExclusionList(Object target, Member member) { + useDevModeConfiguration(); Class memberClass = member.getDeclaringClass(); if (isClassExcluded(memberClass)) { LOG.warn("Declaring class of member type [{}] is excluded!", memberClass); @@ -520,4 +526,36 @@ public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) protected void useDevMode(String devMode) { this.isDevMode = BooleanUtils.toBoolean(devMode); } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false) + public void useDevModeExcludedClasses(String commaDelimitedClasses) { + this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { + this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false) + public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) { + this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) + public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) { + this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses); + } + + private void useDevModeConfiguration() { + if (!isDevMode || isDevModeInit) { + return; + } + isDevModeInit = true; + LOG.warn("Working in devMode, using devMode excluded classes and packages!"); + excludedClasses = devModeExcludedClasses; + excludedPackageNamePatterns = devModeExcludedPackageNamePatterns; + excludedPackageNames = devModeExcludedPackageNames; + excludedPackageExemptClasses = devModeExcludedPackageExemptClasses; + } } 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 b8e100ee9e..27a0d0f330 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -80,6 +80,11 @@ public void setUp() throws Exception { ognlUtil = container.getInstance(OgnlUtil.class); } + private void resetOgnlUtil(Map properties) { + loadButSet(properties); + ognlUtil = container.getInstance(OgnlUtil.class); + } + public void testCanSetADependentObject() { String dogName = "fido"; @@ -1152,8 +1157,8 @@ public void testAvoidCallingMethodsOnObjectClass() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); - ognlUtil.setValue("class.classLoader.defaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); + // Object.class is excluded by default + ognlUtil.setValue("class.classLoader", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { expected = e; @@ -1166,9 +1171,11 @@ public void testAvoidCallingMethodsOnObjectClass() { public void testAllowCallingMethodsOnObjectClassInDevModeTrue() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Foo.class.getName()); - ognlUtil.setDevModeExcludedClasses(""); - ognlUtil.setDevMode(Boolean.TRUE.toString()); + Map properties = new HashMap<>(); + properties.put(StrutsConstants.STRUTS_EXCLUDED_CLASSES, Foo.class.getName()); + properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, ""); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString()); + resetOgnlUtil(properties); Foo foo = new Foo(); String result = (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class); @@ -1180,14 +1187,18 @@ public void testAllowCallingMethodsOnObjectClassInDevModeTrue() { } public void testExclusionListDevModeOnOff() throws Exception { - ognlUtil.setDevModeExcludedClasses(Foo.class.getName()); Foo foo = new Foo(); - ognlUtil.setDevMode(Boolean.TRUE.toString()); + Map properties = new HashMap<>(); + properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, Foo.class.getName()); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString()); + resetOgnlUtil(properties); + OgnlException e = assertThrows(OgnlException.class, () -> ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class)); assertThat(e).hasMessageContaining("com.opensymphony.xwork2.util.Foo.toString"); - ognlUtil.setDevMode(Boolean.FALSE.toString()); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE.toString()); + resetOgnlUtil(properties); assertEquals("Foo", (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class)); } @@ -1196,7 +1207,7 @@ public void testAvoidCallingMethodsOnObjectClassUpperCased() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("Class.ClassLoader.DefaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1212,7 +1223,7 @@ public void testAvoidCallingMethodsOnObjectClassAsMap() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("class['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1243,7 +1254,7 @@ public void testAvoidCallingMethodsOnObjectClassAsMapWithQuotes() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("class[\"classLoader\"]['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1284,12 +1295,11 @@ public void testAvoidCallingMethodsWithBraces() { assertEquals(expected.getMessage(), "Inappropriate OGNL expression: toString()"); } - public void testAvoidCallingSomeClasses() { + public void testStaticMethodBlocked() { Foo foo = new Foo(); Exception expected = null; try { - ognlUtil.setExcludedClasses(Runtime.class.getName()); ognlUtil.setValue("@java.lang.Runtime@getRuntime().exec('mate')", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) {