Disallow nested objects and arrays as keys in objects #772
Conversation
For object keys, we can just skip the part of `nextValue()` that parses values
that are objects or arrays. Then the existing logic for unquoted values will
already stop at `{` or `[`, and that will produce a `Missing value` exception.
|
@eamonnmcmanus I think this is a good fix and probably overdue. However, since JSONTokenizer is central to the lib, more testing would be helpful. For example, the non-quoted value might be an object key, object value, or an array element. The value itself might be a simple string or might contain one or more JSON control chars: backslash, curly brace, bracket, comma, colon, or double quote. It might be number-like (contains one or more periods, hyphens, plus signs, or 'e' chars). It would be good to know if the tokenizer processes these examples in a reasonable way. There might also be other potential tests I haven't thought of. This could get tedious to code and might be more than you signed up for. Let me know if you want some help with the unit tests. Either way, thanks for getting this started. |
|
I've added some extra tests to cover unquoted text. They are not as extensive as listed in @stleary's comment. Further test coverage for unquoted text would surely be good, but I think is a bit out of scope for this PR. The goal here is to fix a DoS problem, while not breaking any existing reasonable uses. I think the tests cover that quite well. |
stleary
changed the title
|
What problem does this code solve? Risks Changes to the API? Will this require a new release? Should the documentation be updated? Does it break the unit tests? Was any code refactored in this commit? Review status Starting 3-day comment window. |
stleary
changed the title
Disallow nested objects and arrays as keys in objects.
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
Fixes #771.