Gouri Sankar A created WW-5758:
----------------------------------
Summary: 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.3.0, 7.0.0, 7.4.0
Reporter: Gouri Sankar A
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)
