[
https://issues.apache.org/jira/browse/WW-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5726:
------------------------------
Component/s: Plugin - REST
> Merged polymorphic property with a non-null initial value is not authorized
> in the REST plugin
> ----------------------------------------------------------------------------------------------
>
> Key: WW-5726
> URL: https://issues.apache.org/jira/browse/WW-5726
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
>
> h2. Summary
> A property that is mergeable ({{@JsonMerge}}, or merge enabled through
> configuration), carries a value type deserializer ({{@JsonTypeInfo}} on the
> property or its declared type), and has a non-null initial value is
> deserialized by Jackson through a path that reaches neither
> {{AuthorizingSettableBeanProperty}} nor {{AuthorizingValueDeserializer}}. The
> property is assigned without an authorization check, and its members are
> authorized against the enclosing prefix instead of their own.
> Sibling of WW-5725, which covers the other {{set()}} and value-deserializer
> entry points; this one cannot be closed from the property wrapper.
> h2. Current behaviour
> {{MergingSettableBeanProperty}} is constructed by
> {{BeanDeserializerBase.resolve()}} around the already-wrapped property, so
> {{BeanDeserializer}} calls its {{deserializeAndSet}}, which never reaches
> {{AuthorizingSettableBeanProperty.deserializeAndSet}}. With a non-null
> current value it calls the {{final}}
> {{SettableBeanProperty.deserializeWith(p, ctxt, oldValue)}}. When
> {{_valueTypeDeserializer}} is set, that method derives the subtype from
> {{oldValue.getClass()}}, resolves a fresh contextual deserializer for it, and
> deserializes in place into {{oldValue}} — bypassing the property's
> {{_valueDeserializer}} (where {{AuthorizingValueDeserializer}} sits). Because
> the merge is in place, {{newValue == oldValue}} and {{set()}} is not called
> either.
> Consequences: the property itself is never authorized, and no path is pushed,
> so the merged bean's members are authorized as {{member}} at the enclosing
> prefix rather than {{property.member}}. A grant on a sibling property of the
> enclosing bean with the same member name therefore authorizes the nested
> write. No type id is required in the body; the subtype comes from the
> existing value.
> The non-polymorphic merge (no type deserializer) goes through
> {{_valueDeserializer.deserialize(p, ctxt, oldValue)}} and is covered by
> WW-5725.
> Applies to JSON and XML bodies alike. 6.x is unaffected: the REST plugin
> there has no Jackson authorization module.
> h2. Proposed change
> Both facts are known when {{ParameterAuthorizingModule.updateBuilder}} runs:
> {{property.getMetadata().getMergeInfo() != null}} and
> {{property.hasValueTypeDeserializer()}}. Refuse the combination there, so the
> definition fails closed rather than binding unchecked — either by reporting a
> bad definition or by stripping the merge for that property so it takes the
> ordinary authorized path.
> The alternative of intercepting the in-place typed deserialize from
> {{RedactionAwareDeserializer.createContextual}} would keep the merge working,
> but it has to avoid a second path push on the normal path and is the more
> fragile of the two.
> Add a test in the same direction as the WW-5725 tests: a {{@JsonMerge}}
> {{@JsonTypeInfo}} property with an initialised field, an authorizer granting
> the enclosing bean's sibling member of the same name, asserting the nested
> member is not assigned.
> h2. Compatibility notes
> An application that relies on merging into a polymorphic property with a
> non-null initial value through the REST plugin will see that definition
> rejected (or merge disabled for it) until the property is given a null
> initial value or the type deserializer is dropped. No other property kind is
> affected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)