This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5525-sma-npe
in repository https://gitbox.apache.org/repos/asf/struts.git
commit a8bce0f94ffb0fe8e0db97012c9989375587eb64
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Sat Feb 1 07:40:18 2025 +0100
WW-5525 Fixes NPE when checking if expressions is acceptable
---
.../apache/struts2/ognl/SecurityMemberAccess.java | 20 +++---
.../struts2/ognl/SecurityMemberAccessTest.java | 1 +
.../ognl/SecurityMemberAccessProxyTest.java | 76 ++++++++++++++++++++++
3 files changed, 87 insertions(+), 10 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
index 9c266645b..6596968a5 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -141,7 +141,7 @@ public class SecurityMemberAccess implements MemberAccess {
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- if (target != null) {
+ if (target != null && member != null) {
// Special case: Target is a Class object but not Class.class
if (Class.class.equals(target.getClass()) &&
!Class.class.equals(target)) {
if (!isStatic(member) &&
!Constructor.class.equals(member.getClass())) {
@@ -157,44 +157,44 @@ public class SecurityMemberAccess implements MemberAccess
{
}
}
- if (!checkProxyObjectAccess(target)) {
+ if (target != null && !checkProxyObjectAccess(target)) {
LOG.warn("Access to proxy is blocked! Target [{}], proxy class
[{}]", target, target.getClass().getName());
return false;
}
- if (!checkProxyMemberAccess(target, member)) {
+ if (target != null && member != null &&
!checkProxyMemberAccess(target, member)) {
LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(member)) {
+ if (member != null && !checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member)) {
+ if (member != null && !checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- if (!checkStaticMethodAccess(member)) {
+ if (member != null && !checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (!checkDefaultPackageAccess(target, member)) {
+ if (target != null && member != null &&
!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (!checkExclusionList(target, member)) {
+ if (target != null && member != null && !checkExclusionList(target,
member)) {
return false;
}
- if (!checkAllowlist(target, member)) {
+ if (target != null && member != null && !checkAllowlist(target,
member)) {
return false;
}
- if (!isAcceptableProperty(propertyName)) {
+ if (propertyName != null && !isAcceptableProperty(propertyName)) {
return false;
}
diff --git
a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
index e3d3fa589..1cd7e9a60 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
@@ -649,6 +649,7 @@ public class SecurityMemberAccessTest {
@Test
public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception {
// given
+ assignNewSma(false);
sma.useExcludedClasses(String.join(",", Class.class.getName(),
StaticTester.class.getName()));
// when
diff --git
a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java
b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java
index 91ffae19e..d6b7230b1 100644
---
a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java
+++
b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java
@@ -87,4 +87,80 @@ public class SecurityMemberAccessProxyTest extends
XWorkJUnit4TestCase {
assertTrue(sma.isAccessible(context, proxy.getAction(),
proxyObjectProxyMember, ""));
assertTrue(sma.isAccessible(context, proxy.getAction(),
proxyObjectNonProxyMember, ""));
}
+
+ @Test
+ public void nullTargetAndTargetAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember,
""));
+ }
+
+ @Test
+ public void nullTargetAndTargetAllowedAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember,
""));
+ }
+
+ @Test
+ public void nullTargetAndTargetAndMemberAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+ assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember,
""));
+ }
+
+ @Test
+ public void nullMemberAndTargetAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertFalse(sma.isAccessible(context, proxy.getAction(), null, ""));
+ }
+
+ @Test
+ public void nullMemberAndTargetAllowedAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertTrue(sma.isAccessible(context, proxy.getAction(), null, ""));
+ }
+
+ @Test
+ public void nullMemberAndTargetNotAllowedAndMemberAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+ assertFalse(sma.isAccessible(context, proxy.getAction(), null, ""));
+ }
+
+ @Test
+ public void nullTargetAndMemberAndTargetAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertTrue(sma.isAccessible(context, null, null, ""));
+ }
+
+ @Test
+ public void nullTargetAndMemberAndTargetNotAllowedAndMemberAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+ assertTrue(sma.isAccessible(context, null, null, ""));
+ }
+
+ @Test
+ public void nullTargetAndMemberAndTargetAllowedAndMemberNotAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+ assertTrue(sma.isAccessible(context, null, null, ""));
+ }
+
+ @Test
+ public void nullTargetAndMemberAndTargetAndMemberAllowed() {
+ sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+ sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+ assertTrue(sma.isAccessible(context, null, null, ""));
+ }
+
+ @Test
+ public void nullPropertyName() {
+ sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+ assertTrue(sma.isAccessible(context, proxy.getAction(),
proxyObjectProxyMember, null));
+ }
}
