Skip to content
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

CVE-2023-5072: disallow nested object/array keys & detect embedded \0 #1

Merged
merged 2 commits into from Apr 1, 2024

Conversation

claireagordon
Copy link
Collaborator

Port stleary/JSON-java fixes for CVE-2023-5072 to mitigate recursion issues when creating JSONObjects.

  • when JSONObject parses keys, use a getSimpleValue tokenizer method that does not support objects or arrays
  • keep track of EOF character '\0' when parsing objects & throw an error if in an unsupported location.

The latter requires us to port over a more modern implementation of JSONTokener.next(int n) (https://github.com/stleary/JSON-java/blob/master/src/main/java/org/json/JSONTokener.java#L248) so we can check individual characters instead of reading an n-character buffer all at once.

Tested by:

  • importing into alpha locally & running all unit tests that depend on //thirdparty:json. Confirm all test failures match the main branch
  • Running the Snyk Proof-of-concept for this CVE & confirming JSONObject creation throws an exception instead of throwing an OutOfMemory error due to recursion

See also:

Port of stleary/JSON-java#772
to partially remediate
https://www.cve.org/CVERecord?id=CVE-2023-5072 , where
nested keys can allow relatively small inputs to
cause OOM errors through recursion.

Test by:
- package & import into alpha locally
- confirm a suite of unit tests depending on JSONObjects
passes
- verify that the following CVE Proof-of-concept fails
with an 'unexpected character' exception:
https://security.snyk.io/vuln/SNYK-JAVA-ORGJSON-5962464
See:
stleary/JSON-java#758
stleary/JSON-java#759

Port pull #759 from stleary/JSON-java to help
address OOM errors described in
https://www.cve.org/CVERecord?id=CVE-2023-5072

To support the JSONTokener.end() function this
relies on, port over the 'eof' flag & set in
all locations it's used in the latest JSON-java.

Use the String next(int n) implementation from
more recent java versions so we can properly check
end() while reading a group of characters.

Test by:
- importing into alpha locally & running all tests
that depend on //thirdparty:json
- verifying that Snyk's proof-of-concept does
not cause OOMs:
https://security.snyk.io/vuln/SNYK-JAVA-ORGJSON-5962464
Copy link

@jfancher-yext jfancher-yext left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@claireagordon claireagordon merged commit 0d32b4a into main Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants