[
https://issues.apache.org/jira/browse/WW-5676?focusedWorklogId=1037281&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1037281
]
ASF GitHub Bot logged work on WW-5676:
--------------------------------------
Author: ASF GitHub Bot
Created on: 23/Aug/26 16:28
Start Date: 23/Aug/26 16:28
Worklog Time Spent: 10m
Work Description: lukaszlenart merged PR #1858:
URL: https://github.com/apache/struts/pull/1858
Issue Time Tracking
-------------------
Worklog Id: (was: 1037281)
Time Spent: 0.5h (was: 20m)
> Decide whether array and primitive types should resolve to their
> element/wrapper package in OGNL security checks
> ----------------------------------------------------------------------------------------------------------------
>
> Key: WW-5676
> URL: https://issues.apache.org/jira/browse/WW-5676
> Project: Struts 2
> Issue Type: Improvement
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Split out of [WW-5674|https://issues.apache.org/jira/browse/WW-5674], where
> the question was deliberately deferred so that ticket could remain a pure
> no-behaviour-change optimisation.
> h2. Current behaviour
> {{SecurityMemberAccess.toPackageName(Class)}} returns the empty string for
> arrays, primitives and {{void}}:
> {code:java}
> public static String toPackageName(Class clazz) {
> if (clazz.isArray() || clazz.isPrimitive()) {
> return "";
> }
> return clazz.getPackageName();
> }
> {code}
> (The real signature is generic; generics are omitted here because the issue
> tracker mangles them.)
> The {{isArray() || isPrimitive()}} guard exists specifically to preserve
> this. It reproduces the pre-WW-5674 implementation, which used
> {{clazz.getPackage()}} — and {{getPackage()}} returns null for exactly those
> three categories.
> {{Class.getPackageName()}} resolves them differently: arrays resolve to the
> element type's package and primitives to {{java.lang}}. Verified on Temurin
> 17:
> || class || current {{toPackageName}} || {{getPackageName()}} ||
> | {{String[]}} | {{""}} | {{java.lang}} |
> | {{java.io.File[]}} | {{""}} | {{java.io}} |
> | {{int}}, {{int[]}}, {{void}} | {{""}} | {{java.lang}} |
> | {{com.app.MyThing[]}} | {{""}} | {{com.app}} |
> h2. Why this is not a simple hardening fix
> {{toPackageName}} feeds two checks that the change would push in _opposite_
> directions.
> _The exclusion path would tighten._ {{isExcludedPackageNames}} would start
> matching arrays. A {{java.io.File[]}} currently has package {{""}} and
> escapes {{struts.excludedPackageNames}} entirely, even though {{java.io}} is
> excluded by default. That looks like a genuine defensive gap.
> _The allowlist path would loosen._ {{isClassAllowlisted}} would start
> matching arrays too. An application that sets
> {{struts.allowlist.packageNames=com.app.actions}} does _not_ today thereby
> allowlist {{com.app.actions.MyThing[]}}, because the array's package is
> {{""}}; arrays must be listed explicitly in {{struts.allowlist.classes}}.
> After the change they would be allowlisted implicitly.
> The allowlist is the primary OGNL defence in Struts 7.x and is enabled by
> default ({{struts.allowlist.enable=true}}), so making it more permissive
> needs its own security reasoning rather than riding along with a performance
> change.
> h2. What needs deciding
> * Whether the exclusion-path gap (arrays of excluded-package types escaping
> exclusion) is exploitable in practice. Note that for an array target,
> {{member.getDeclaringClass()}} is usually {{java.lang.Object}}, which is
> already in {{struts.excludedClasses}}; {{clone()}} is the notable exception,
> as its declaring class is the array type itself.
> * Whether the allowlist loosening is acceptable, or whether the two paths
> should use different package semantics.
> * Whether {{void}} and primitives should be treated separately from arrays —
> they can never be an OGNL target in the same way.
> * Whether any change here needs a migration-guide note, since it can break
> applications that rely on the current behaviour in either direction.
> h2. Constraints
> Any change must keep {{checkDefaultPackageAccess}} consistent — it
> independently inspects {{getPackage()}} and would otherwise disagree with
> {{toPackageName}} about what "the default package" means. See WW-5677, which
> proposes routing that method through {{toPackageName}}.
> Requires equivalence tests in {{SecurityMemberAccessPackageMatchingTest}},
> which already pins the current array/primitive behaviour in
> {{arraysAndPrimitivesResolveToTheEmptyPackage}} and
> {{toPackageNameMatchesLegacyAcrossClassShapes}}. Those tests will need
> updating deliberately, not incidentally.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
