Skip to content

Commit 2a8a686

Browse files
committed
Blocks ognl access to class members of Spring proxy
1 parent 843693f commit 2a8a686

File tree

3 files changed

+58
-3
lines changed

3 files changed

+58
-3
lines changed

core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java

+26-2
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
*/
1616
package com.opensymphony.xwork2.ognl;
1717

18+
import com.opensymphony.xwork2.util.ProxyUtil;
1819
import ognl.DefaultMemberAccess;
20+
import org.apache.commons.lang3.reflect.FieldUtils;
21+
import org.apache.commons.lang3.reflect.MethodUtils;
1922
import org.apache.logging.log4j.LogManager;
2023
import org.apache.logging.log4j.Logger;
2124

22-
import java.lang.reflect.Member;
23-
import java.lang.reflect.Modifier;
25+
import java.lang.reflect.*;
2426
import java.util.Collections;
2527
import java.util.Map;
2628
import java.util.Set;
@@ -85,6 +87,11 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
8587
return false;
8688
}
8789

90+
if (isProxyAccess(target, member)) {
91+
LOG.warn("Access to proxy [{}] is blocked!", member);
92+
return false;
93+
}
94+
8895
boolean allow = true;
8996
if (!checkStaticMethodAccess(member)) {
9097
LOG.warn("Access to static [{}] is blocked!", member);
@@ -100,6 +107,23 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
100107
return super.isAccessible(context, target, member, propertyName) && isAcceptableProperty(propertyName);
101108
}
102109

110+
protected boolean isProxyAccess(Object target, Member member) {
111+
if (!ProxyUtil.isSpringAopProxy(target))
112+
return false;
113+
Class<?> clazz = ProxyUtil.springUltimateTargetClass(target);
114+
if (member instanceof Method) {
115+
return null == MethodUtils.getMatchingMethod(clazz, member.getName(), ((Method) member).getParameterTypes());
116+
}
117+
if (member instanceof Field) {
118+
return null == FieldUtils.getField(clazz, member.getName(), true);
119+
}
120+
if (member instanceof Constructor) {
121+
return false;
122+
}
123+
124+
return true;
125+
}
126+
103127
protected boolean checkStaticMethodAccess(Member member) {
104128
int modifiers = member.getModifiers();
105129
if (Modifier.isStatic(modifiers)) {

core/src/test/java/com/opensymphony/xwork2/spring/ActionsFromSpringTest.java

+27
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@
55

66
import com.opensymphony.xwork2.*;
77
import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider;
8+
import org.apache.commons.lang3.reflect.MethodUtils;
9+
import org.apache.struts2.dispatcher.HttpParameters;
810
import org.springframework.context.ApplicationContext;
911

12+
import java.util.HashMap;
13+
import java.util.Map;
14+
1015
/**
1116
* Test loading actions from the Spring Application Context.
1217
*
@@ -100,4 +105,26 @@ public void testChainingProxiedActions() throws Exception {
100105
assertEquals(1, chaintoAOPedAction.getCount());
101106
assertEquals("WW-4105", chaintoAOPedAction.getName());
102107
}
108+
109+
public void testProxiedActionIsNotAccessible() throws Exception {
110+
// given
111+
Map<String, Object> params = new HashMap<>();
112+
params.put("exposeProxy", "true");
113+
params.put("issueId", "S2-047");
114+
115+
HashMap<String, Object> extraContext = new HashMap<>();
116+
extraContext.put(ActionContext.PARAMETERS, HttpParameters.create(params).build());
117+
118+
ActionProxy proxy = actionProxyFactory.createActionProxy(null,
119+
"chaintoAOPedTestSubBeanAction", null, extraContext);
120+
121+
// when
122+
proxy.execute();
123+
Object action = proxy.getAction();
124+
125+
//then
126+
assertEquals("S2-047", ((TestSubBean) action).getIssueId());
127+
assertFalse("proxied action is accessible!",
128+
(boolean) MethodUtils.invokeMethod(action, "isExposeProxy"));
129+
}
103130
}

core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml

+5-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212

1313
<interceptors>
1414
<interceptor name="chain"
15-
class="com.opensymphony.xwork2.interceptor.ChainingInterceptor"></interceptor>
15+
class="com.opensymphony.xwork2.interceptor.ChainingInterceptor"/>
16+
<interceptor name="params"
17+
class="com.opensymphony.xwork2.interceptor.ParametersInterceptor"/>
1618
</interceptors>
1719

1820
<action name="simpleAction" class="simple-action"/>
@@ -36,7 +38,9 @@
3638
<action name="chaintoAOPedTestSubBeanAction" class="pointcutted-test-sub-bean"
3739
method="getIssueId">
3840
<interceptor-ref name="chain" />
41+
<interceptor-ref name="params" />
3942
<result name="WW-4105" type="null" />
43+
<result name="S2-047" type="null" />
4044
</action>
4145
</package>
4246
</xwork>

0 commit comments

Comments
 (0)