Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WW-5342 Add option to block use of default package #742

Merged
merged 3 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
1 change: 1 addition & 0 deletions core/src/main/java/org/apache/struts2/StrutsConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down