[
https://issues.apache.org/jira/browse/WW-5288?focusedWorklogId=848499&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-848499
]
ASF GitHub Bot logged work on WW-5288:
--------------------------------------
Author: ASF GitHub Bot
Created on: 02/Mar/23 04:08
Start Date: 02/Mar/23 04:08
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #664:
URL: https://github.com/apache/struts/pull/664#discussion_r1122577804
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -197,51 +208,61 @@ protected boolean checkEnumAccess(Object target, Member
member) {
return false;
}
- protected boolean isPackageExcluded(Package targetPackage, Package
memberPackage) {
- if (targetPackage == null || memberPackage == null) {
- LOG.warn("The use of the default (unnamed) package is
discouraged!");
+ protected boolean isPackageExcluded(Class<?> targetClass, Class<?>
memberClass) {
+ if (targetClass == null || memberClass == null) {
+ throw new IllegalArgumentException(
+ "Parameters should never be null - if member is static,
targetClass should be the same as memberClass.");
}
- String targetPackageName = targetPackage == null ? "" :
targetPackage.getName();
- String memberPackageName = memberPackage == null ? "" :
memberPackage.getName();
+ Set<Class<?>> classesToCheck = new HashSet<>();
+ classesToCheck.add(targetClass);
+ classesToCheck.add(memberClass);
- for (Pattern pattern : excludedPackageNamePatterns) {
- if (pattern.matcher(targetPackageName).matches() ||
pattern.matcher(memberPackageName).matches()) {
+ for (Class<?> clazz : classesToCheck) {
+ if (!isExcludedPackageExempt(clazz) &&
(isExcludedPackageNamePatterns(clazz) || isExcludedPackageNames(clazz))) {
Review Comment:
Exemption must now exist for both classes (target and member) if they both
match a banned package
Issue Time Tracking
-------------------
Worklog Id: (was: 848499)
Time Spent: 20m (was: 10m)
> Make excluded package exemption logic more strict
> -------------------------------------------------
>
> Key: WW-5288
> URL: https://issues.apache.org/jira/browse/WW-5288
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Reporter: Kusal Kithul-Godage
> Priority: Minor
> Fix For: 6.2.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Following on from the discussion in the comments on WW-5268 - exempting
> classes from excluded packages should only be done if unavoidable.
> Given this, I realised we should make the exemption logic more strict to
> prevent incorrect use and inadvertent exempting of more OGNL expressions than
> intended.
> * Currently, the exempted classes also match against superclasses. This is
> unnecessary and we can match against only the specific class.
> * Currently, an exemption against either the target or member class suffices.
> This can be made more strict by requiring an exemption for the class which
> matches the excluded package specifically, which could be either or both.
> * The JavaDoc for the options should be very explicit in what each
> configuration option achieves to prevent incorrect uses.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
