[
https://issues.apache.org/jira/browse/WW-5829?focusedWorklogId=1051640&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1051640
]
ASF GitHub Bot logged work on WW-5829:
--------------------------------------
Author: ASF GitHub Bot
Created on: 31/Oct/26 14:37
Start Date: 31/Oct/26 14:37
Worklog Time Spent: 10m
Work Description: g0w6y opened a new pull request, #2087:
URL: https://github.com/apache/struts/pull/2087
Fixes [WW-5829](https://issues.apache.org/jira/browse/WW-5829)
## What & why
`ParametersInterceptor` hands only `Parameter.getValue()` (the first value)
to `ParameterValueAware.acceptableParameterValue` and to the excluded/accepted
value patterns, but then sets the whole `String[]` on the action. So
`?tags=ok&tags=evil` passes a filter that rejects `evil`, while `?tags=evil`
and `?tags=evil&tags=ok` are rejected.
`JSONInterceptor` already checks every element of a JSON array (WW-4858).
This checks every value of a repeated form parameter too, with one difference:
JSONInterceptor drops only the bad list element, while here one rejected value
rejects the whole parameter.
## Change
`isAcceptableParameterValue` and `isAcceptableParameterValueAware` now check
every value of a multi-valued parameter (small private helper `valuesToCheck`).
One rejected value rejects the whole parameter, which is what already happens
when the first value is rejected. Single-valued and empty parameters go through
the same calls with the same argument as before.
## How verified
Three tests added to `ParametersInterceptorTest` (excluded patterns,
accepted patterns, `ParameterValueAware`), each covering single value, all
values good, first value bad, second value bad.
- Without the change: `Tests run: 48, Failures: 3` (all three new tests:
`secondBad` reached the stack).
- With the change: `ParametersInterceptorTest` (48),
`ParameterAuthorizerTest` (42) and `StrutsParameterAnnotationTest` (33) pass.
- Also checked against a running Struts Boot app with an action implementing
`ParameterValueAware` that rejects `evil`: `?tags=ok&tags=evil` was accepted
before (the action received `[ok, evil]`) and is rejected with the change;
`?tags=ok&tags=fine` still binds.
## Not changed
- No new configuration; value filters stay opt-in.
- No partial filtering: a bad value does not just drop that one value
(JSONInterceptor drops the bad list element; here the parameter is rejected
whole, as it already is for a bad first value).
- Parameter name checks are untouched.
## Risk
Behaviour changes only for actions that configure value patterns or
implement `ParameterValueAware` and receive repeated parameters where a later
value is rejected; those requests were previously (wrongly) accepted. Cost is
one extra pattern match per additional value.
Issue Time Tracking
-------------------
Worklog Id: (was: 1051640)
Remaining Estimate: 0h
Time Spent: 10m
> ParametersInterceptor value filters only check the first value of a repeated
> parameter
> --------------------------------------------------------------------------------------
>
> Key: WW-5829
> URL: https://issues.apache.org/jira/browse/WW-5829
> Project: Struts 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 6.12.0, 7.0.0, 7.3.0, 7.4.0
> Reporter: Gouri Sankar A
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> ParametersInterceptor checks a parameter's value with
> ParameterValueAware.acceptableParameterValue(...) and with
> excludedValuePatterns / acceptedValuePatterns, but passes only
> Parameter.getValue(), which for a repeated parameter is the first value. The
> whole String[] is then set on the action.
> Reproduction (main, against a running Struts Boot app): an action
> implementing ParameterValueAware that rejects "evil", with a String[] setter
> annotated @StrutsParameter.
> ?tags=evil -> rejected
> ?tags=evil&tags=ok -> rejected
> ?tags=ok&tags=evil -> accepted, the action receives [ok, evil]
> The same holds for excludedValuePatterns and acceptedValuePatterns.
> The ParameterValueAware javadoc says it lets an action reject values "to
> prevent clients from setting other unexpected (and possibly dangerous)
> parameter values". JSONInterceptor already checks every element of a JSON
> array (WW-4858).
> Affected versions: the same check (param.getValue(), which is the first
> value) is present in the 6.12.0, 7.0.0, 7.3.0 and 7.4.0 tags, by source
> inspection. The behaviour itself was only reproduced on main.
> Fix: check every value of a repeated parameter. One rejected value rejects
> the whole parameter, as a rejected first value does today. Single-valued and
> empty parameters are checked as before.
> Tests: three new tests in ParametersInterceptorTest (excluded patterns,
> accepted patterns, ParameterValueAware). They fail without the change and
> pass with it.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
