[
https://issues.apache.org/jira/browse/WW-5747?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5747:
------------------------------
Description:
h2. Summary
{{RedactionAwareDeserializer}} wraps every bean deserializer in the REST
plugin's Jackson authorization and, when construction fails after a
{{@StrutsParameter}} redaction marked its scope, catches the
{{JsonMappingException}} and returns {{null}} so the whole object is dropped
rather than partially exposed. It returns without moving the parser: whatever
tokens of the dropped object remain unread are then consumed by the _enclosing_
bean's property loop.
h2. Current behaviour
Any failure that fires before the dropped object's last field desyncs the read.
Two shapes reach it:
* a creator-bound bean whose construction fails when its last creator parameter
arrives (a redacted primitive under {{FAIL_ON_NULL_FOR_PRIMITIVES}}, a compact
constructor rejecting {{null}}) while non-creator fields follow it in the body;
* a bean whose property-based {{@JsonIdentityInfo}} id is a bean that itself
failed to construct on a redacted member: the {{null}} id fails Jackson's
binding at the {{id}} property (WW-5746).
Reproduced with this fixture, {{child.id.k}} denied and every other path
granted:
{code:java}public record StrictKey(String k) {
public StrictKey {
Objects.requireNonNull(k);
}
}
@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class,
property = "id")
public class StrictIdentified {
public StrictKey id;
public String name;
}
public class StrictHolder {
public StrictIdentified child;
public String other;
public String name;
}
{code}
{code:json}{"child": {"id": {"k": "a"}, "name": "x"}, "other": "o"}
{code}
{{child}} is {{null}} as intended, but the child's {{name}} value {{x}} was
bound to the _holder's_ {{name}}, and the holder's own {{other}} was lost
because its object read ended at the child's closing brace. Without a
same-named property on the parent the stray field raises "Unrecognized field",
which — the parent's scope being marked redacted — silently drops the whole
root object instead.
The misplaced value still passes the parent property's own {{@StrutsParameter}}
check at its own path, so this is data landing in the wrong property, not an
authorization bypass.
h2. Proposed change
Resync before returning {{null}}: record the parser's nesting depth (and
whether the entry token was {{START_OBJECT}}, {{START_ARRAY}} or a
{{FIELD_NAME}} inside the object) when the wrapper is entered, and on a
swallowed failure skip tokens — {{skipChildren()}} on any nested start token —
until the matching {{END_OBJECT}} or {{END_ARRAY}} at that depth has been
consumed. A scalar entry token needs no resync. Cover both shapes above with
tests, asserting the parent's later properties still bind and the dropped
object's fields never reach the parent.
h2. Compatibility notes
No configuration or API change. Only bodies that already trigger an object drop
are affected, and only in that the enclosing object is now read correctly
afterwards.
was:
h2. Summary
{{RedactionAwareDeserializer}} wraps every bean deserializer in the REST
plugin's Jackson authorization and, when construction fails after a
{{@StrutsParameter}} redaction marked its scope, catches the
{{JsonMappingException}} and returns {{null}} so the whole object is dropped
rather than partially exposed. It returns without moving the parser: whatever
tokens of the dropped object remain unread are then consumed by the _enclosing_
bean's property loop.
h2. Current behaviour
Any failure that fires before the dropped object's last field desyncs the read.
Two shapes reach it:
* a creator-bound bean whose construction fails when its last creator parameter
arrives (a redacted primitive under {{FAIL_ON_NULL_FOR_PRIMITIVES}}, a compact
constructor rejecting {{null}}) while non-creator fields follow it in the body;
* a bean whose property-based {{@JsonIdentityInfo}} id is a bean that itself
failed to construct on a redacted member: the {{null}} id fails Jackson's
binding at the {{id}} property (WW-5746).
Reproduced with {{record StrictKey(String k)}} (rejects {{null}}) as the id of
{{StrictIdentified { id; name }}}, held in {{StrictHolder { child; other; name
}}}, body {{{"child":{"id":{"k":"a"},"name":"x"},"other":"o"}}} and
{{child.id.k}} denied: {{child}} is {{null}} as intended, but the child's
{{name}} value {{"x"}} was bound to the *holder's* {{name}}, and the holder's
own {{other}} was lost because its object read ended at the child's closing
brace. Without a same-named property on the parent the stray field raises
"Unrecognized field", which — the parent's scope being marked redacted —
silently drops the whole root object instead.
The misplaced value still passes the parent property's own {{@StrutsParameter}}
check at its own path, so this is data landing in the wrong property, not an
authorization bypass.
h2. Proposed change
Resync before returning {{null}}: record the parser's nesting depth (and
whether the entry token was {{START_OBJECT}}, {{START_ARRAY}} or a
{{FIELD_NAME}} inside the object) when the wrapper is entered, and on a
swallowed failure skip tokens — {{skipChildren()}} on any nested start token —
until the matching {{END_OBJECT}}/{{END_ARRAY}} at that depth has been
consumed. A scalar entry token needs no resync. Cover both shapes above with
tests, asserting the parent's later properties still bind and the dropped
object's fields never reach the parent.
h2. Compatibility notes
No configuration or API change. Only bodies that already trigger an object drop
are affected, and only in that the enclosing object is now read correctly
afterwards.
> RedactionAwareDeserializer leaves the parser mid-object when it drops a REST
> body object
> ----------------------------------------------------------------------------------------
>
> Key: WW-5747
> URL: https://issues.apache.org/jira/browse/WW-5747
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
>
> h2. Summary
> {{RedactionAwareDeserializer}} wraps every bean deserializer in the REST
> plugin's Jackson authorization and, when construction fails after a
> {{@StrutsParameter}} redaction marked its scope, catches the
> {{JsonMappingException}} and returns {{null}} so the whole object is dropped
> rather than partially exposed. It returns without moving the parser: whatever
> tokens of the dropped object remain unread are then consumed by the
> _enclosing_ bean's property loop.
> h2. Current behaviour
> Any failure that fires before the dropped object's last field desyncs the
> read. Two shapes reach it:
> * a creator-bound bean whose construction fails when its last creator
> parameter arrives (a redacted primitive under
> {{FAIL_ON_NULL_FOR_PRIMITIVES}}, a compact constructor rejecting {{null}})
> while non-creator fields follow it in the body;
> * a bean whose property-based {{@JsonIdentityInfo}} id is a bean that itself
> failed to construct on a redacted member: the {{null}} id fails Jackson's
> binding at the {{id}} property (WW-5746).
> Reproduced with this fixture, {{child.id.k}} denied and every other path
> granted:
> {code:java}public record StrictKey(String k) {
> public StrictKey {
> Objects.requireNonNull(k);
> }
> }
> @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class,
> property = "id")
> public class StrictIdentified {
> public StrictKey id;
> public String name;
> }
> public class StrictHolder {
> public StrictIdentified child;
> public String other;
> public String name;
> }
> {code}
> {code:json}{"child": {"id": {"k": "a"}, "name": "x"}, "other": "o"}
> {code}
> {{child}} is {{null}} as intended, but the child's {{name}} value {{x}} was
> bound to the _holder's_ {{name}}, and the holder's own {{other}} was lost
> because its object read ended at the child's closing brace. Without a
> same-named property on the parent the stray field raises "Unrecognized
> field", which — the parent's scope being marked redacted — silently drops the
> whole root object instead.
> The misplaced value still passes the parent property's own
> {{@StrutsParameter}} check at its own path, so this is data landing in the
> wrong property, not an authorization bypass.
> h2. Proposed change
> Resync before returning {{null}}: record the parser's nesting depth (and
> whether the entry token was {{START_OBJECT}}, {{START_ARRAY}} or a
> {{FIELD_NAME}} inside the object) when the wrapper is entered, and on a
> swallowed failure skip tokens — {{skipChildren()}} on any nested start token
> — until the matching {{END_OBJECT}} or {{END_ARRAY}} at that depth has been
> consumed. A scalar entry token needs no resync. Cover both shapes above with
> tests, asserting the parent's later properties still bind and the dropped
> object's fields never reach the parent.
> h2. Compatibility notes
> No configuration or API change. Only bodies that already trigger an object
> drop are affected, and only in that the enclosing object is now read
> correctly afterwards.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
