[
https://issues.apache.org/jira/browse/WW-5624?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5624:
------------------------------
Description:
h2. Summary
{{@StrutsParameter}} enforcement is currently implemented in
{{ParametersInterceptor}} for standard request parameter binding, but
request-body based binders in some plugins bypass that authorization model and
populate action/model objects directly.
This creates inconsistent behavior between URL/form parameters and JSON/XML
request bodies, and may allow mass assignment of properties that would normally
be rejected by {{ParametersInterceptor}}.
h2. Affected areas currently identified
* JSON plugin:
{{JSONPopulator.populateObject()}} sets properties via direct reflection and
does not follow the full {{@StrutsParameter}} authorization rules.
* REST plugin:
{{JacksonJsonHandler.toObject()}} updates target objects directly via Jackson
and does not follow the full {{@StrutsParameter}} authorization rules.
h2. Problem scope
The issue is broader than checking whether a setter is annotated. The current
core contract in {{ParametersInterceptor}} also includes:
* permitted nesting depth
* authorization based on the exposed root member
* ModelDriven handling
* transition mode semantics
* related allowlisting behavior
Any request-body binding implementation should align with that same contract,
otherwise Struts applies different security rules depending on how input
reaches the action/model.
h2. Expected direction
Instead of implementing separate partial checks in each plugin, Struts should
reuse or extract the shared parameter-binding authorization logic from
{{ParametersInterceptor}} and apply it consistently across request-body binders.
was:
h2. Summary
{{JSONPopulator.populateObject()}} in the struts2-json-plugin sets action
properties via direct Java reflection ({{{}Method.invoke(){}}}) without
checking the {{@StrutsParameter}} annotation. This bypasses the parameter
allowlisting that {{ParametersInterceptor}} enforces for URL parameters,
enabling mass assignment of unannotated properties via JSON request body.
{{JacksonJsonHandler.toObject()}} in the struts2-rest-plugin uses
{{ObjectMapper.readerForUpdating(target).readValue(reader)}} to merge JSON
request body directly into the action object. Jackson sets any field with a
matching setter, completely bypassing the {{@StrutsParameter}} annotation check
that {{ParametersInterceptor}} enforces for URL parameters. This enables mass
assignment of unannotated properties via REST JSON request body.
h2. Changes
* {*}JSONPopulator.java{*}: Add {{@StrutsParameter}} annotation check before
{{{}Method.invoke(){}}}. When {{struts.parameters.requireAnnotations}} is
enabled and the setter lacks the annotation, the property is skipped with a
debug log message.
* {*}JSONInterceptor.java{*}: Wire the
{{struts.parameters.requireAnnotations}} setting via {{@Inject}} into
{{{}JSONPopulator.setRequireAnnotations(){}}}.
* {*}JacksonJsonHandler.java{*}: When {{struts.parameters.requireAnnotations}}
is enabled, deserialize JSON into a map first, then filter properties against
the {{@StrutsParameter}} annotation on the target class's setter methods before
setting them. When disabled, preserve the original {{readerForUpdating()}}
merge for backwards compatibility.
h2. Impact
Without this fix:
* URL params to unannotated setters: *BLOCKED* (ParametersInterceptor enforces)
* JSON body to same unannotated setters: *BYPASSES* (JSONPopulator ignores
annotation)
With this fix, both pathways consistently enforce {{{}@StrutsParameter{}}}.
h2. REFER
[https://github.com/apache/struts/pull/1651]
[https://github.com/apache/struts/pull/1652]
[https://github.com/apache/struts/pull/1651/changes/03a07b871ac82d8955b310155b3cbe1e30c861b3]
https://github.com/apache/struts/pull/1652/changes/d4b01caded8453bc3d02bd6ecff72e264dc81dd0
> Request body population bypasses @StrutsParameter contract outside
> ParametersInterceptor
> ----------------------------------------------------------------------------------------
>
> Key: WW-5624
> URL: https://issues.apache.org/jira/browse/WW-5624
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - JSON, Plugin - REST
> Affects Versions: 7.1.1
> Reporter: Tran Quac
> Priority: Major
> Fix For: 7.2.0
>
>
> h2. Summary
> {{@StrutsParameter}} enforcement is currently implemented in
> {{ParametersInterceptor}} for standard request parameter binding, but
> request-body based binders in some plugins bypass that authorization model
> and populate action/model objects directly.
> This creates inconsistent behavior between URL/form parameters and JSON/XML
> request bodies, and may allow mass assignment of properties that would
> normally be rejected by {{ParametersInterceptor}}.
> h2. Affected areas currently identified
> * JSON plugin:
> {{JSONPopulator.populateObject()}} sets properties via direct reflection and
> does not follow the full {{@StrutsParameter}} authorization rules.
> * REST plugin:
> {{JacksonJsonHandler.toObject()}} updates target objects directly via Jackson
> and does not follow the full {{@StrutsParameter}} authorization rules.
> h2. Problem scope
> The issue is broader than checking whether a setter is annotated. The current
> core contract in {{ParametersInterceptor}} also includes:
> * permitted nesting depth
> * authorization based on the exposed root member
> * ModelDriven handling
> * transition mode semantics
> * related allowlisting behavior
> Any request-body binding implementation should align with that same contract,
> otherwise Struts applies different security rules depending on how input
> reaches the action/model.
> h2. Expected direction
> Instead of implementing separate partial checks in each plugin, Struts should
> reuse or extract the shared parameter-binding authorization logic from
> {{ParametersInterceptor}} and apply it consistently across request-body
> binders.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
