fix(rest-plugin): WW-5624 enforce @StrutsParameter in JacksonJsonHandler.toObject()#1652
fix(rest-plugin): WW-5624 enforce @StrutsParameter in JacksonJsonHandler.toObject()#1652tranquac wants to merge 1 commit intoapache:mainfrom
Conversation
…ject() JacksonJsonHandler.toObject() uses ObjectMapper.readerForUpdating(target) to merge JSON request body directly into the action object, bypassing the @StrutsParameter annotation check that ParametersInterceptor enforces for URL parameters. This allows mass assignment of unannotated properties via REST JSON request body. When struts.parameters.requireAnnotations is enabled, deserialize JSON into a map first, then filter properties by @StrutsParameter annotation on the target's setter methods before setting them. Only annotated setters are populated, consistent with ParametersInterceptor behavior. When requireAnnotations is disabled, preserve the original unrestricted readerForUpdating() merge for backwards compatibility.
|
Thanks for reporting this. The issue in the REST plugin looks valid: That said, I don't think this fix is sufficient as-is. In Struts, My main concern is that once an annotated top-level setter is accepted, I think the right direction is to align the REST plugin with the shared parameter-binding authorization rules used by |
|
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. |
Summary
JacksonJsonHandler.toObject()in the struts2-rest-plugin usesObjectMapper.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@StrutsParameterannotation check thatParametersInterceptorenforces for URL parameters. This enables mass assignment of unannotated properties via REST JSON request body.Changes
struts.parameters.requireAnnotationsis enabled, deserialize JSON into a map first, then filter properties against the@StrutsParameterannotation on the target class's setter methods before setting them. When disabled, preserve the originalreaderForUpdating()merge for backwards compatibility.Impact
Without this fix:
With this fix, both pathways consistently enforce
@StrutsParameter.Test
A PoC application with 9 test cases demonstrates the bypass using an
OrderActionwith annotated (setItemName,setQuantity) and unannotated (setUnitPrice,setDiscount,setApproved,setInternalNote) setters. JSON body setsunitPricefrom 99.99 to 0.01 andapprovedto true before the fix.