Merge pull request #742 from apache/WW-5342-default-package

WW-5342 Add option to block use of default package
apache · Sep 26, 2023 · bb83a60 · bb83a60
2 parents 501d395 + bfe1f8c
commit bb83a60
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 4 deletions.
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -92,6 +92,7 @@ public class OgnlUtil {
     private Container container;
     private boolean allowStaticFieldAccess = true;
     private boolean disallowProxyMemberAccess;
+    private boolean disallowDefaultPackageAccess;
 
     /**
      * Construct a new OgnlUtil instance for use with the framework
@@ -287,6 +288,11 @@ protected void setDisallowProxyMemberAccess(String disallowProxyMemberAccess) {
         this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess);
     }
 
+    @Inject(value = StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, required = false)
+    protected void setDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) {
+        this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
+    }
+
     /**
      * @param maxLength Injects the Struts OGNL expression maximum length.
      */
@@ -310,6 +316,10 @@ public boolean isDisallowProxyMemberAccess() {
         return disallowProxyMemberAccess;
     }
 
+    public boolean isDisallowDefaultPackageAccess() {
+        return disallowDefaultPackageAccess;
+    }
+
     /**
      * Convenience mechanism to clear the OGNL Runtime Cache via OgnlUtil.  May be utilized
      * by applications that generate many unique OGNL expressions over time.

diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
@@ -94,6 +94,7 @@ protected void setOgnlUtil(OgnlUtil ognlUtil) {
         securityMemberAccess.useExcludedPackageNames(ognlUtil.getExcludedPackageNames());
         securityMemberAccess.useExcludedPackageExemptClasses(ognlUtil.getExcludedPackageExemptClasses());
         securityMemberAccess.disallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess());
+        securityMemberAccess.disallowDefaultPackageAccess(ognlUtil.isDisallowDefaultPackageAccess());
     }
 
     protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, boolean allowStaticFieldAccess) {

diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -56,6 +56,7 @@ public class SecurityMemberAccess implements MemberAccess {
     private Set<String> excludedPackageNames = emptySet();
     private Set<String> excludedPackageExemptClasses = emptySet();
     private boolean disallowProxyMemberAccess;
+    private boolean disallowDefaultPackageAccess;
 
     /**
      * SecurityMemberAccess
@@ -143,13 +144,19 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
         }
 
         if (targetClass != memberClass && isClassExcluded(targetClass)) {
-            // Optimization: Already checked memberClass exclusion, so if-and-only-if targetClass == memberClass, this check is redundant.
             LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
             return false;
         }
 
-        if (targetClass.getPackage() == null || memberClass.getPackage() == null) {
-            LOG.warn("The use of the default (unnamed) package is discouraged!");
+        if (disallowDefaultPackageAccess) {
+            if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
+                LOG.warn("Class [{}] from the default package is excluded!", targetClass);
+                return false;
+            }
+            if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
+                LOG.warn("Class [{}] from the default package is excluded!", memberClass);
+                return false;
+            }
         }
 
         if (isPackageExcluded(targetClass)) {
@@ -160,7 +167,7 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
             return false;
         }
 
-        if (isPackageExcluded(memberClass)) {
+        if (targetClass != memberClass && isPackageExcluded(memberClass)) {
             LOG.warn("Package [{}] of member [{}] are excluded!", memberClass.getPackage(), member);
             return false;
         }
@@ -374,4 +381,8 @@ public void setDisallowProxyMemberAccess(boolean disallowProxyMemberAccess) {
     public void disallowProxyMemberAccess(boolean disallowProxyMemberAccess) {
         this.disallowProxyMemberAccess = disallowProxyMemberAccess;
     }
+
+    public void disallowDefaultPackageAccess(boolean disallowDefaultPackageAccess) {
+        this.disallowDefaultPackageAccess = disallowDefaultPackageAccess;
+    }
 }
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -448,6 +448,7 @@ public final class StrutsConstants {
     public static final String STRUTS_LOCALIZED_TEXT_PROVIDER = "struts.localizedTextProvider";
 
     public static final String STRUTS_DISALLOW_PROXY_MEMBER_ACCESS = "struts.disallowProxyMemberAccess";
+    public static final String STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS = "struts.disallowDefaultPackageAccess";
 
     public static final String STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT = "struts.ognl.autoGrowthCollectionLimit";
 

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -315,6 +315,16 @@ public void testDefaultPackageExclusion() throws Exception {
         assertFalse("default package is excluded!", actual);
     }
 
+    @Test
+    public void testDefaultPackageExclusionSetting() throws Exception {
+        sma.disallowDefaultPackageAccess(true);
+
+        Class<?> clazz = Class.forName("PackagelessAction");
+        boolean actual = sma.isAccessible(null, clazz.getConstructor().newInstance(), clazz.getMethod("execute"), null);
+
+        assertFalse("default package isn't excluded!", actual);
+    }
+
     @Test
     public void testDefaultPackageExclusion2() throws Exception {
         // given