[
https://issues.apache.org/jira/browse/WW-5758?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5758:
------------------------------
Fix Version/s: 7.5.0
> 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
> Fix For: 7.5.0
>
> 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)
