Skip to content

fix(json-plugin): WW-5624 enforce @StrutsParameter in JSONPopulator.populateObject()#1651

Open
tranquac wants to merge 1 commit intoapache:mainfrom
tranquac:fix/json-plugin-strutsparameter-bypass
Open

fix(json-plugin): WW-5624 enforce @StrutsParameter in JSONPopulator.populateObject()#1651
tranquac wants to merge 1 commit intoapache:mainfrom
tranquac:fix/json-plugin-strutsparameter-bypass

Conversation

@tranquac
Copy link
Copy Markdown

@tranquac tranquac commented Apr 6, 2026

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.

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().

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.

Test

A PoC application with 8 test cases demonstrates the bypass and fix. An action with @StrutsParameter-annotated setter (setUsername) and unannotated setters (setRole, setAdmin) shows that JSON body sets unannotated fields before the fix, and blocks them after.

…bject()

JSONPopulator.populateObject() uses Method.invoke() to set action
properties from JSON input, bypassing the @StrutsParameter annotation
check that ParametersInterceptor enforces for URL parameters. This
allows mass assignment of unannotated properties via JSON request body.

Add @StrutsParameter annotation check before Method.invoke() in
JSONPopulator, gated by the existing struts.parameters.requireAnnotations
setting. When enabled, only setters annotated with @StrutsParameter are
populated from JSON input, consistent with ParametersInterceptor.

Wire the requireAnnotations setting from StrutsConstants into
JSONInterceptor -> JSONPopulator via @Inject.
@lukaszlenart
Copy link
Copy Markdown
Member

Could you create a JIRA ticket first? I can include also changes in #1652

@tranquac
Copy link
Copy Markdown
Author

tranquac commented Apr 8, 2026

I requested a Jira account!

And i will create create a JIRA ticket soon!

@tranquac
Copy link
Copy Markdown
Author

tranquac commented Apr 8, 2026

Hi @lukaszlenart

I created JIRA ticket for this issue: https://issues.apache.org/jira/projects/WW/issues/WW-5624?filter=allissues

@lukaszlenart lukaszlenart changed the title fix(json-plugin): enforce @StrutsParameter in JSONPopulator.populateObject() fix(json-plugin): WW-5624 enforce @StrutsParameter in JSONPopulator.populateObject() Apr 8, 2026
@lukaszlenart
Copy link
Copy Markdown
Member

Thanks for reporting this. The issue in the JSON plugin looks valid: JSONPopulator.populateObject() currently bypasses the ParametersInterceptor checks for @StrutsParameter.

That said, I don't think this fix is sufficient as-is. In Struts, @StrutsParameter support is not only a check for an annotated setter. The core ParametersInterceptor logic also enforces nesting depth and related authorization semantics. This change currently adds only a setter-level annotation check, so it does not fully match existing Struts behavior.

My main concern is that JSONPopulator recursively populates nested beans, collections, arrays, and maps. Requiring @StrutsParameter on every nested setter would be stricter than current Struts semantics, where authorization is based on the exposed root member and the permitted depth. That could break valid nested binding scenarios while still not aligning fully with the core parameter-binding contract.

I think the right direction is to align the JSON plugin with the shared parameter-binding authorization rules used by ParametersInterceptor, instead of implementing a local setter-only check.

@tranquac
Copy link
Copy Markdown
Author

tranquac commented Apr 8, 2026

Thanks for your feedback. I will quickly create a fullfill patch based on your comment. I will notify you when it's finished and create a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants