Skip to content

Commit 9fee06c

Browse files
committed
WW-5525 Fixes NPE when checking if expressions is acceptable
1 parent 31c3fc5 commit 9fee06c

File tree

3 files changed

+90
-5
lines changed

3 files changed

+90
-5
lines changed

core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,12 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
160160
}
161161
}
162162

163-
if (!checkProxyObjectAccess(target)) {
163+
if (target != null && !checkProxyObjectAccess(target)) {
164164
LOG.warn("Access to proxy is blocked! Target [{}], proxy class [{}]", target, target.getClass().getName());
165165
return false;
166166
}
167167

168-
if (!checkProxyMemberAccess(target, member)) {
168+
if (target != null && !checkProxyMemberAccess(target, member)) {
169169
LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
170170
return false;
171171
}
@@ -185,15 +185,15 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
185185
return false;
186186
}
187187

188-
if (!checkDefaultPackageAccess(target, member)) {
188+
if (target != null && !checkDefaultPackageAccess(target, member)) {
189189
return false;
190190
}
191191

192-
if (!checkExclusionList(target, member)) {
192+
if (target != null && !checkExclusionList(target, member)) {
193193
return false;
194194
}
195195

196-
if (!checkAllowlist(target, member)) {
196+
if (target != null && !checkAllowlist(target, member)) {
197197
return false;
198198
}
199199

core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,7 @@ public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception {
649649
@Test
650650
public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception {
651651
// given
652+
assignNewSma(false);
652653
sma.useExcludedClasses(String.join(",", Class.class.getName(), StaticTester.class.getName()));
653654

654655
// when

plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Map;
3232

3333
import static org.junit.Assert.assertFalse;
34+
import static org.junit.Assert.assertThrows;
3435
import static org.junit.Assert.assertTrue;
3536

3637
public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase {
@@ -87,4 +88,87 @@ public void allowAllProxyAccess() {
8788
assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, ""));
8889
assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectNonProxyMember, ""));
8990
}
91+
92+
@Test
93+
public void nullTargetAndTargetAndMemberNotAllowed() {
94+
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
95+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
96+
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
97+
}
98+
99+
@Test
100+
public void nullTargetAndTargetAllowedAndMemberNotAllowed() {
101+
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
102+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
103+
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
104+
}
105+
106+
@Test
107+
public void nullTargetAndTargetAndMemberAllowed() {
108+
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
109+
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
110+
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
111+
}
112+
113+
@Test
114+
public void nullMemberAndTargetAndMemberNotAllowed() {
115+
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
116+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
117+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
118+
() -> sma.isAccessible(context, proxy.getAction(), null, ""));
119+
}
120+
121+
@Test
122+
public void nullMemberAndTargetAllowedAndMemberNotAllowed() {
123+
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
124+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
125+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
126+
() -> sma.isAccessible(context, proxy.getAction(), null, ""));
127+
}
128+
129+
@Test
130+
public void nullMemberAndTargetNotAllowedAndMemberAllowed() {
131+
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
132+
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
133+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
134+
() -> sma.isAccessible(context, proxy.getAction(), null, ""));
135+
}
136+
137+
@Test
138+
public void nullTargetAndMemberAndTargetAndMemberNotAllowed() {
139+
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
140+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
141+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
142+
() -> sma.isAccessible(context, null, null, ""));
143+
}
144+
145+
@Test
146+
public void nullTargetAndMemberAndTargetNotAllowedAndMemberAllowed() {
147+
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
148+
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
149+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
150+
() -> sma.isAccessible(context, null, null, ""));
151+
}
152+
153+
@Test
154+
public void nullTargetAndMemberAndTargetAllowedAndMemberNotAllowed() {
155+
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
156+
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
157+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
158+
() -> sma.isAccessible(context, null, null, ""));
159+
}
160+
161+
@Test
162+
public void nullTargetAndMemberAndTargetAndMemberAllowed() {
163+
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
164+
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
165+
assertThrows("Member cannot be null!", IllegalArgumentException.class,
166+
() -> sma.isAccessible(context, null, null, ""));
167+
}
168+
169+
@Test
170+
public void nullPropertyName() {
171+
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
172+
assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, null));
173+
}
90174
}

0 commit comments

Comments
 (0)