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 4d552d5948..10fdf65ced 100644 --- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java @@ -310,6 +310,15 @@ protected boolean checkExclusionList(Object target, Member member) { } /** + * Blocks access to classes in the default (unnamed) package. + *

+ * The emptiness of {@link #toPackageName(Class)} is the same test as the + * {@code getPackage() == null || getPackage().getName().isEmpty()} form this replaced, for every + * class shape: {@code getPackage()} is null for arrays, primitives and {@code void}, and names the + * unnamed package with the empty string, all of which {@code toPackageName} reports as empty. It + * avoids the {@code getPackage()} lookup through the defining classloader's package map, which + * ran twice per class here. See WW-5677. + * * @return {@code true} if member access is allowed */ protected boolean checkDefaultPackageAccess(Object target, Member member) { @@ -317,7 +326,7 @@ protected boolean checkDefaultPackageAccess(Object target, Member member) { return true; } Class memberClass = member.getDeclaringClass(); - if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) { + if (toPackageName(memberClass).isEmpty()) { LOG.warn("Class [{}] from the default package is excluded!", memberClass); return false; } @@ -325,7 +334,7 @@ protected boolean checkDefaultPackageAccess(Object target, Member member) { return true; } Class targetClass = target.getClass(); - if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) { + if (toPackageName(targetClass).isEmpty()) { LOG.warn("Class [{}] from the default package is excluded!", targetClass); return false; } @@ -407,7 +416,9 @@ public static String toPackageName(Class clazz) { } protected boolean isExcludedPackageNamePatterns(Class clazz) { - return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(toPackageName(clazz)).matches()); + // Resolved once rather than inside the lambda, which re-resolved it per configured pattern. + String packageName = toPackageName(clazz); + return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(packageName).matches()); } protected boolean isExcludedPackageNames(Class clazz) { diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java index 3dfa540dd4..d333b20af6 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java @@ -53,6 +53,15 @@ private static boolean legacyPrefixMatch(String packageName, Set matchin .anyMatch(matchingPackages::contains); } + /** + * The default-package condition replaced by WW-5677 in {@code checkDefaultPackageAccess}, + * retained verbatim as the reference oracle. Deliberately calls {@link Class#getPackage()} + * directly rather than delegating to production code, so that it cannot drift with it. + */ + private static boolean legacyDefaultPackageCondition(Class clazz) { + return clazz.getPackage() == null || clazz.getPackage().getName().isEmpty(); + } + /** * The {@code toPackageName} implementation replaced by WW-5674, retained as the reference oracle. */ @@ -168,6 +177,21 @@ public void toPackageNameMatchesLegacyAcrossClassShapes() throws Exception { } } + /** + * WW-5677 replaced {@code getPackage() == null || getPackage().getName().isEmpty()} in + * {@code checkDefaultPackageAccess} with {@code toPackageName(clazz).isEmpty()}. That gate + * decides whether a class counts as living in the default package, so the equivalence is + * asserted against the frozen oracle over every class shape rather than argued. + */ + @Test + public void defaultPackageConditionMatchesLegacyAcrossClassShapes() throws Exception { + for (Class clazz : classShapes()) { + assertThat(toPackageName(clazz).isEmpty()) + .as("default-package condition for %s", clazz.getName()) + .isEqualTo(legacyDefaultPackageCondition(clazz)); + } + } + @Test public void arraysAndPrimitivesResolveToTheEmptyPackage() { assertThat(toPackageName(int.class)).isEmpty(); 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 0338636316..4bfb3c2267 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java @@ -404,6 +404,33 @@ public void testDefaultPackageExclusionSetting() throws Exception { assertFalse("default package isn't excluded!", actual); } + /** + * WW-5677 routed {@code checkDefaultPackageAccess} through {@code toPackageName}. A class in a + * named package must still pass the gate when the setting is on. + */ + @Test + public void testDefaultPackageAccessPermitsNamedPackageClass() throws Exception { + sma.useDisallowDefaultPackageAccess(Boolean.TRUE.toString()); + + Member member = FooBar.class.getMethod("getStringField"); + + assertTrue("a named-package class is blocked!", sma.checkDefaultPackageAccess(new FooBar(), member)); + } + + /** + * WW-5677 equivalence at the shape most likely to break it: an array target reports the empty + * package under both the old {@code getPackage() == null} form and the new one, so it must stay + * blocked. The member here declares in {@code java.lang}, so only the target branch can block. + */ + @Test + public void testDefaultPackageAccessBlocksArrayTarget() throws Exception { + sma.useDisallowDefaultPackageAccess(Boolean.TRUE.toString()); + + Member member = Object.class.getMethod("toString"); + + assertFalse("an array target isn't blocked!", sma.checkDefaultPackageAccess(new String[0], member)); + } + @Test public void testDefaultPackageExclusion2() throws Exception { // given