From 086b63735527d4bb0c1dd0d86a7c0374b825ff24 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Fri, 7 Jul 2017 13:35:10 +0430 Subject: [PATCH] Adds constant to control proxy member access --- .../src/main/resources/struts-plugin.xml | 1 + .../opensymphony/xwork2/XWorkConstants.java | 1 + .../opensymphony/xwork2/ognl/OgnlUtil.java | 11 +++++ .../xwork2/ognl/OgnlValueStack.java | 1 + .../xwork2/ognl/SecurityMemberAccess.java | 7 ++- .../ognl/SecurityMemberAccessProxyTest.java | 49 +++++++++++++++++++ .../xwork2/spring/actionContext-xwork.xml | 1 + 7 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java diff --git a/plugins/spring/src/main/resources/struts-plugin.xml b/plugins/spring/src/main/resources/struts-plugin.xml index 2e9b1b16c..8f4685810 100644 --- a/plugins/spring/src/main/resources/struts-plugin.xml +++ b/plugins/spring/src/main/resources/struts-plugin.xml @@ -34,6 +34,7 @@ + diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java index bc532d0d5..b0c274826 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java @@ -28,4 +28,5 @@ public final class XWorkConstants { public static final String OVERRIDE_EXCLUDED_PATTERNS = "overrideExcludedPatterns"; public static final String OVERRIDE_ACCEPTED_PATTERNS = "overrideAcceptedPatterns"; + public static final String XWORK_DISALLOW_PROXY_MEMBER_ACCESS = "xwork.disallowProxyMemberAccess"; } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 42132ba16..e1cc46e75 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -72,6 +72,7 @@ public class OgnlUtil { private Container container; private boolean allowStaticMethodAccess; + private boolean disallowProxyMemberAccess; @Inject public void setXWorkConverter(XWorkConverter conv) { @@ -144,6 +145,15 @@ public class OgnlUtil { this.allowStaticMethodAccess = Boolean.parseBoolean(allowStaticMethodAccess); } + @Inject(value = XWorkConstants.XWORK_DISALLOW_PROXY_MEMBER_ACCESS, required = false) + public void setDisallowProxyMemberAccess(String disallowProxyMemberAccess) { + this.disallowProxyMemberAccess = Boolean.parseBoolean(disallowProxyMemberAccess); + } + + public boolean isDisallowProxyMemberAccess() { + return disallowProxyMemberAccess; + } + /** * Sets the object's properties using the default type converter, defaulting to not throw * exceptions for problems setting the properties. @@ -654,6 +664,7 @@ public class OgnlUtil { memberAccess.setExcludedClasses(excludedClasses); memberAccess.setExcludedPackageNamePatterns(excludedPackageNamePatterns); memberAccess.setExcludedPackageNames(excludedPackageNames); + memberAccess.setDisallowProxyMemberAccess(disallowProxyMemberAccess); return Ognl.createDefaultContext(root, resolver, defaultConverter, memberAccess); } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index 3f441697a..f6decf39d 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java @@ -83,6 +83,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS securityMemberAccess.setExcludedClasses(ognlUtil.getExcludedClasses()); securityMemberAccess.setExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns()); securityMemberAccess.setExcludedPackageNames(ognlUtil.getExcludedPackageNames()); + securityMemberAccess.setDisallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess()); } protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 6ff74f142..7d52a46fa 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -42,6 +42,7 @@ public class SecurityMemberAccess extends DefaultMemberAccess { private Set> excludedClasses = Collections.emptySet(); private Set excludedPackageNamePatterns = Collections.emptySet(); private Set excludedPackageNames = Collections.emptySet(); + private boolean disallowProxyMemberAccess; public SecurityMemberAccess(boolean method) { super(false); @@ -94,7 +95,7 @@ public class SecurityMemberAccess extends DefaultMemberAccess { return false; } - if (ProxyUtil.isProxyMember(member, target)) { + if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) { LOG.warn("Access to proxy [#0] is blocked!", member); return false; } @@ -222,4 +223,8 @@ public class SecurityMemberAccess extends DefaultMemberAccess { public void setExcludedPackageNames(Set excludedPackageNames) { this.excludedPackageNames = excludedPackageNames; } + + public void setDisallowProxyMemberAccess(boolean disallowProxyMemberAccess) { + this.disallowProxyMemberAccess = disallowProxyMemberAccess; + } } diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java new file mode 100644 index 000000000..7e11ceb25 --- /dev/null +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java @@ -0,0 +1,49 @@ +package com.opensymphony.xwork2.ognl; + +import java.lang.reflect.Member; +import java.util.HashMap; +import java.util.Map; + +import com.opensymphony.xwork2.ActionProxy; +import com.opensymphony.xwork2.XWorkTestCase; +import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; + +public class SecurityMemberAccessProxyTest extends XWorkTestCase { + private Map context; + + @Override + public void setUp() throws Exception { + super.setUp(); + + context = new HashMap(); + // Set up XWork + XmlConfigurationProvider provider = new XmlConfigurationProvider("com/opensymphony/xwork2/spring/actionContext-xwork.xml"); + container.inject(provider); + loadConfigurationProviders(provider); + } + + public void testProxyAccessIsBlocked() throws Exception { + ActionProxy proxy = actionProxyFactory.createActionProxy(null, + "paramsAwareProxiedAction", null, context); + + SecurityMemberAccess sma = new SecurityMemberAccess(false); + sma.setDisallowProxyMemberAccess(true); + + Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); + + boolean accessible = sma.isAccessible(context, proxy.getAction(), member, ""); + assertFalse(accessible); + } + + public void testProxyAccessIsAccessible() throws Exception { + ActionProxy proxy = actionProxyFactory.createActionProxy(null, + "paramsAwareProxiedAction", null, context); + + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); + + boolean accessible = sma.isAccessible(context, proxy.getAction(), member, ""); + assertTrue(accessible); + } +} \ No newline at end of file diff --git a/xwork-core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml b/xwork-core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml index 928d37f31..88b78ec9a 100644 --- a/xwork-core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml +++ b/xwork-core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml @@ -2,6 +2,7 @@ +