[
https://issues.apache.org/jira/browse/WW-5758?focusedWorklogId=1045754&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1045754
]
ASF GitHub Bot logged work on WW-5758:
--------------------------------------
Author: ASF GitHub Bot
Created on: 05/Oct/26 15:21
Start Date: 05/Oct/26 15:21
Worklog Time Spent: 10m
Work Description: g0w6y opened a new pull request, #1991:
URL: https://github.com/apache/struts/pull/1991
Fixes [WW-5758](https://issues.apache.org/jira/browse/WW-5758)
`AliasInterceptor` copies a value from one named source to a
developer-chosen target property, falling back to the raw HTTP request
parameter of the source name when the source does not resolve on the stack (its
own documented behavior). That fallback value was never checked against
`@StrutsParameter`, unlike every other binding path (`ParametersInterceptor`,
`ChainingInterceptor`, `CookieInterceptor`, the REST/JSON plugins): an alias
target that is not annotated could still be set, with the value sourced
directly from an attacker-controlled request parameter when the fallback fires.
Injects the same shared `ParameterAuthorizer` `ParametersInterceptor` and
`CookieInterceptor` already use, and checks it for the alias target, but only
when its value came from the raw-parameter fallback. A source name that
resolves directly on the stack names something an earlier, properly-authorized
bind already produced; copying that between properties is the same category of
operation `ChainingInterceptor` already performs without requiring annotations
by default, and stays unaffected. No new configuration flag: this follows the
same `struts.parameters.requireAnnotations` default (true since 7.0) the rest
of the framework already uses.
An annotated alias target, reached through the same fallback, is unaffected.
One regression test fails without the change:
`AliasInterceptorTest#testUnannotatedAliasTargetIsRejected`, using a dedicated
test action and XML fixture kept separate from the widely-shared `SimpleAction`
fixture so it cannot affect, or be affected by, the other suites that reuse it.
It proves all three cases together in one request: an unannotated target via
the fallback is rejected, an annotated target via the same fallback still
binds, and an unannotated target reached via a stack-resolved name (no
fallback) still binds.
Issue Time Tracking
-------------------
Worklog Id: (was: 1045754)
Remaining Estimate: 0h
Time Spent: 10m
> Enforce @StrutsParameter authorization in AliasInterceptor's raw-parameter
> fallback
> -----------------------------------------------------------------------------------
>
> Key: WW-5758
> URL: https://issues.apache.org/jira/browse/WW-5758
> Project: Struts 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 7.0.0, 7.3.0, 7.4.0
> Reporter: Gouri Sankar A
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> AliasInterceptor copies a value from one named source to a developer-chosen
> target property, falling
> back to the raw HTTP request parameter of the source name when the source
> does not resolve on the stack
> (its own documented behavior, shown in its own javadoc example). That
> fallback value was never checked
> against @StrutsParameter before being written to the target, unlike every
> other binding path in the
> framework: ParametersInterceptor, ChainingInterceptor, CookieInterceptor, and
> the REST/JSON plugins (my
> own earlier finding, WW-5642, already merged) all call into the shared
> ParameterAuthorizer before
> binding. AliasInterceptor never did. alias is in defaultStack, so it runs on
> every request to any
> default-configured action.
> Tested on main at a235259 (7.5.0-SNAPSHOT), embedded Tomcat 10.1.34, stock
> struts-default, an action with
> one @StrutsParameter-annotated property and one not. Binding the unannotated
> property directly
> (?secret=DIRECT_ATTEMPT) is correctly rejected, stays untouched. The same
> property, reached by
> configuring aliases to map a raw HTTP parameter name onto it and sending that
> parameter instead
> (?rawParam=PWNED_VIA_ALIAS), is not rejected and takes the attacker supplied
> value directly.
> The fix is deliberately narrower than checking every alias target.
> AliasInterceptor has two different
> value sources: a name that resolves directly on the stack, which names state
> an earlier, properly
> authorized bind already produced such as an earlier action in a chain, and
> the raw HTTP parameter
> fallback, which is the only path that carries never authorized external
> input. Checking every target
> unconditionally would have also blocked the first, legitimate category. I
> built that broader version
> first, live tested it, found it silently broke a same request stack to stack
> copy with no HTTP parameter
> involved at all, and narrowed the check to apply only when the value came
> from the fallback, matching
> exactly what ChainingInterceptor already exempts by default for the same
> underlying reason.
> Affects 7.0.0 through 7.4.0, verified directly on all three released versions
> by reading the code at each
> tag. requireAnnotations has been the default since the very first 7.0 release
> and AliasInterceptor has
> never checked it. Does not affect 6.x: requireAnnotations is not set in 6.x's
> default.properties at all,
> confirmed directly on 6.12.0, so there is no default on control for any
> interceptor to bypass on that
> line.
> This is the same bug class as WW-5642, already merged: a secondary binding
> path that does not enforce
> @StrutsParameter the way ParametersInterceptor does.
> I have a patch with a regression test ready and will open a PR referencing
> this ticket.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
