Lukasz Lenart created WW-5726:
---------------------------------
Summary: 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
Reporter: Lukasz Lenart
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)
