[
https://issues.apache.org/jira/browse/WW-5725?focusedWorklogId=1040961&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1040961
]
ASF GitHub Bot logged work on WW-5725:
--------------------------------------
Author: ASF GitHub Bot
Created on: 11/Sep/26 14:43
Start Date: 11/Sep/26 14:43
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1916:
URL: https://github.com/apache/struts/pull/1916
Fixes [WW-5725](https://issues.apache.org/jira/browse/WW-5725)
`AuthorizingSettableBeanProperty` authorized a property in
`deserializeAndSet` / `deserializeSetAndReturn` and wrapped the value
deserializer only for creator-bound properties. Jackson takes neither route for
a non-creator property it buffers during property-based creation: a setter
property that appears in the body before the last creator parameter is read
through the `final` `SettableBeanProperty.deserialize()` with no authorization
and no path push, and assigned after construction through
`SettableBeanProperty.set()`, which the `Delegating` base forwarded unchecked.
The same property after the last creator parameter, or on a setter-only type,
was already rejected, so member order alone decided whether the check applied,
and the members of a buffered bean-valued property were checked one level too
shallow.
Three gates now cover the paths between them:
- `deserializeAndSet` / `deserializeSetAndReturn` keep authorizing the
direct path and skipping the value on rejection, so the setter never fires, but
no longer push the path themselves.
- `AuthorizingValueDeserializer` wraps every property's value deserializer,
not only creator properties, and owns the path push for nested members. It
covers all three of Jackson's entry points — `deserialize(p, ctxt)`, the
in-place `deserialize(p, ctxt, intoValue)` used for setterless collections, and
`deserializeWithType` for polymorphic properties — and classifies the `[0]`
element prefix on the property's declared type. On the direct path it re-checks
a path `deserializeAndSet` already accepted; the authorizer is stateless, so
the answer is the same.
- `set` / `setAndReturn` authorize the already-materialized assignment,
which also covers a buffered `null` (Jackson skips the value deserializer for a
null token) and the other callers of `set()` in jackson-databind: `@JsonMerge`,
`@JsonManagedReference`, inner-class valued properties, `EXTERNAL_PROPERTY`
type ids and the `@JsonIdentityInfo` id property, none of which were authorized
before.
All gates go through `DynamicKeyAuthorizationContext`, so a dynamic-key
scope authorizes by depth on every path.
**Tests** (all in `ParameterAuthorizingModuleTest`): setter before and after
the last creator parameter, a setter-only type with the same member order, a
nested creator, a creator-plus-setter type inside a dynamic-key scope, a
buffered bean-valued setter whose members must be authorized at their own
depth, a setterless collection, and a polymorphic property. The last two are
written in the direction that matters — a sibling grant on the enclosing bean
must not authorize a collapsed nested path — and both failed against an
intermediate version of this change that had moved the push but not yet covered
the 3-arg and typed entry points.
**Behaviour notes.** No configuration or API change. A request that
previously had an unauthorized property assigned only because of its position
in the body now has it dropped, consistent with every other position.
Properties assigned through the other `set()` callers listed above are now
subject to the same authorization as ordinary setters — most visibly, an
`@JsonIdentityInfo` id property now needs `@StrutsParameter` like any other. A
buffered rejection logs twice (once at the read, once at the assignment); the
assignment-side warning is kept because it is the only one for the other
`set()` callers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1040961)
Remaining Estimate: 0h
Time Spent: 10m
> AuthorizingSettableBeanProperty does not authorize the buffered set() path in
> the REST plugin
> ---------------------------------------------------------------------------------------------
>
> Key: WW-5725
> URL: https://issues.apache.org/jira/browse/WW-5725
> Project: Struts 2
> Issue Type: Bug
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Summary
> {{AuthorizingSettableBeanProperty}} authorizes a property in
> {{deserializeAndSet}} and {{deserializeSetAndReturn}}, and wraps the value
> deserializer for creator-bound properties. It does not override {{set(Object,
> Object)}} or {{setAndReturn(Object, Object)}}, which is the path Jackson
> takes for a non-creator property it has buffered during property-based
> creation. That property is then assigned without an authorization check.
> h2. Current behaviour
> On a type that mixes a property-based creator ({{@JsonCreator}} constructor
> or a record) with an ordinary setter, jackson-databind buffers a setter
> property that appears in the JSON before the last creator parameter,
> constructs the instance, and assigns the buffered value through
> {{PropertyValue.Regular.assign}} → {{SettableBeanProperty.set()}}.
> {{SettableBeanProperty.Delegating.set()}} forwards straight to the delegate,
> so the authorization in {{AuthorizingSettableBeanProperty}} never runs for
> it. {{withValueDeserializer}} is scoped to {{CreatorProperty}}, so the setter
> property keeps its plain value deserializer, and
> {{SettableBeanProperty.deserialize}} is {{final}} and cannot be intercepted.
> The same property is rejected when it appears after the last creator
> parameter (direct {{deserializeAndSet}} path), and on a setter-only type with
> the same payload. The order of members in the request body alone decides
> whether the check applies. Nothing is logged when the buffered assignment
> goes through.
> Applies to JSON and XML bodies alike, since {{JacksonXmlHandler}} registers
> the same module. 6.x is unaffected: the REST plugin there has no Jackson
> authorization module.
> h2. Proposed change
> Override {{set(Object, Object)}} and {{setAndReturn(Object, Object)}} in
> {{AuthorizingSettableBeanProperty}} and apply the same authorization used by
> {{deserializeAndSet}}, dropping the value and logging the rejection the same
> way. That closes the buffered path with no new configuration: the guard
> already rejects the identical property in the other member order, so no
> legitimate behaviour depends on the gap.
> Add tests covering a creator-plus-setter type with the unauthorized setter
> before and after the last creator parameter, top-level and nested, for both
> handlers.
> h2. Compatibility notes
> No configuration or API change. A request that previously had an unauthorized
> property assigned only because of its position in the body now has it
> dropped, consistent with every other position.
> Reported by 0xCc.zhang.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
