-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix #5962: Case-insensitive deserialization may use wrong @JsonIgnoreProperties
#5964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
cowtowncoder
merged 1 commit into
3.1
from
tatu-claude/3.1/5962-bean-deser-base-ci-ignore-props
May 6, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
...est/java/tools/jackson/databind/deser/filter/IgnorePropertiesCaseInsensitive5962Test.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package tools.jackson.databind.deser.filter; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonFormat; | ||
| import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
|
|
||
| import tools.jackson.databind.*; | ||
| import tools.jackson.databind.testutil.DatabindTestUtil; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| /** | ||
| * [databind#5962]: Case-insensitive BeanPropertyMap rebuild undoes per-property | ||
| * {@code @JsonIgnoreProperties}. | ||
| * | ||
| * {@code BeanDeserializerBase.createContextual()} calls {@code _handleByNameInclusion()} | ||
| * to filter properties according to per-property {@code @JsonIgnoreProperties}, producing | ||
| * a contextual deserializer with the restricted {@code BeanPropertyMap}. However, the | ||
| * subsequent case-insensitivity block read {@code _beanProperties} (the *original* | ||
| * unfiltered map from {@code this}) rather than {@code contextual._beanProperties} (the | ||
| * filtered map). {@code withCaseInsensitivity()} then rebuilt the map from the unfiltered | ||
| * source, and {@code contextual.withBeanProperties(props)} overwrote the filtered map with | ||
| * the unfiltered one — any properties removed by {@code _handleByNameInclusion} were | ||
| * restored. | ||
| * | ||
| * Patch: source the case-insensitive rebuild from {@code contextual._beanProperties}. | ||
| */ | ||
| public class IgnorePropertiesCaseInsensitive5962Test extends DatabindTestUtil | ||
| { | ||
| static class AdminDto { | ||
| public String adminKey = "DEFAULT"; | ||
| public String username; | ||
| } | ||
|
|
||
| // Container that ignores "adminKey" on the AdminDto field AND enables case-insensitive matching | ||
| static class Container { | ||
| @JsonIgnoreProperties("adminKey") | ||
| @JsonFormat(with = JsonFormat.Feature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) | ||
| public AdminDto admin; | ||
| } | ||
|
|
||
| // Baseline container: only @JsonIgnoreProperties, no case-insensitive format override | ||
| static class BaselineContainer { | ||
| @JsonIgnoreProperties("adminKey") | ||
| public AdminDto admin; | ||
| } | ||
|
|
||
| /** | ||
| * NEGATIVE CONTROL: without the @JsonFormat case-insensitive override, @JsonIgnoreProperties | ||
| * correctly suppresses adminKey on the nested AdminDto field. | ||
| */ | ||
| @Test | ||
| public void test5962_negativeControl_withoutCaseInsensitivity() throws Exception { | ||
| ObjectMapper mapper = jsonMapperBuilder().build(); | ||
| String json = "{\"admin\":{\"adminKey\":\"HACKED\",\"username\":\"alice\"}}"; | ||
| BaselineContainer result = mapper.readValue(json, BaselineContainer.class); | ||
| // Without case-insensitive format, @JsonIgnoreProperties blocks adminKey | ||
| assertNotEquals("HACKED", result.admin.adminKey, | ||
| "@JsonIgnoreProperties alone (no case-insensitive format) should block adminKey"); | ||
| assertEquals("alice", result.admin.username); | ||
| } | ||
|
|
||
| /** | ||
| * EXPLOIT PATH: the case-insensitive BeanPropertyMap rebuild (triggered by | ||
| * @JsonFormat ACCEPT_CASE_INSENSITIVE_PROPERTIES) restores the unfiltered original | ||
| * _beanProperties, undoing the @JsonIgnoreProperties("adminKey") exclusion. | ||
| * Case-insensitive matching then routes "adminKey" (or "ADMINKEY") to the setter. | ||
| * | ||
| * Security assertion: adminKey must NOT be settable via JSON when the enclosing | ||
| * container declares @JsonIgnoreProperties("adminKey") on the field. | ||
| */ | ||
| @Test | ||
| public void test5962_caseInsensitiveRebuildRestoresIgnoredProperty() throws Exception { | ||
| ObjectMapper mapper = jsonMapperBuilder().build(); | ||
|
|
||
| // Exact case — should be blocked by @JsonIgnoreProperties | ||
| String json = "{\"admin\":{\"adminKey\":\"HACKED\",\"username\":\"alice\"}}"; | ||
| Container result = mapper.readValue(json, Container.class); | ||
| assertNotEquals("HACKED", result.admin.adminKey, | ||
| "[databind#5962]: case-insensitive BeanPropertyMap rebuild restored 'adminKey' " + | ||
| "after it was removed by @JsonIgnoreProperties. The property was set to 'HACKED'."); | ||
| assertEquals("alice", result.admin.username); | ||
|
|
||
| // Mixed case — exploits the case-insensitive rebuild more directly | ||
| String jsonMixed = "{\"admin\":{\"AdminKey\":\"HACKED2\",\"username\":\"bob\"}}"; | ||
| Container result2 = mapper.readValue(jsonMixed, Container.class); | ||
| assertNotEquals("HACKED2", result2.admin.adminKey, | ||
| "[databind#5962]: 'AdminKey' (mixed case) matched 'adminKey' via case-insensitive " + | ||
| "lookup that was rebuilt from the unfiltered property map."); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by GHSA-5jmj-h7xm-6q6v, this test fails for me with Jackson 2.21.4, suggesting that version is also affected, yet the advisory says only >= 3.1.0, < 3.1.4 is affected. What am I missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was specifically reported against 3.1 -- unlike other problems that included 2.21 (and often 2.18). I assumed reported had analyzed these cases; perhaps they did not?
If test can be made to fail against 2.21 branch (or maybe even 2.18), I'd be happy to address there too. Note: PR against 2.18 is likely forward mergeable to 2.21 so one pr would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sonatype now flags it: #5962 (comment)