Repository navigation
Confusion between \0 and EOF can lead to OutOfMemoryError #758
Description
JSONTokener.next() uses a 0 return to indicate EOF. But 0 is also returned when an actual \0 character is read. In some circumstances that can be used to circumvent parser checks. Parsing untrusted input could then potentially lead to OutOfMemoryError even for quite small input strings.
Activity
johnjaylward commentedon Aug 1, 2023
Do you have an actual test to validate this?
I'd argue that valid text input should never have a
\0in it in the first place, so treating it the same as EOF would just make the parsed JSON document invalid. From what I remember of the parser, this should make the parsing stop and if the JSON value is not complete, than the parse would fail with an JSONExceptioneamonnmcmanus commentedon Aug 1, 2023
Yes, I have a test that can provoke OutOfMemoryError with a small input, though I'm not going to publish it until a fix has been released. Yes,
\0should mean EOF everywhere, but it currently doesn't.johnjaylward commentedon Aug 1, 2023
Without a PoC, I doubt anyone else will fix this.
Feel free to submit a PR.
I don't think I'd consider a OOME a security issue, but if you feel different, follow the instructions for those here:
https://github.com/stleary/JSON-java/wiki/FAQ#how-are-vulnerabilities-and-exploits-handled
eamonnmcmanus commentedon Aug 1, 2023
I think Denial of Service attacks kind of are security issues, aren't they? If you can make a server fall over by firing carefully-crafted JSON at it, that seems not great. Anyway, yes, my next step was to submit a PR.
(I have already shared repro details with @stleary.)
Don't skip past `\0` when parsing JSON objects.
stleary commentedon Aug 1, 2023
@eamonnmcmanus Good catch and thanks for bringing this up. In this case, it's OK to post the recreate code and proposed fix.
johnjaylward commentedon Aug 1, 2023
DoS issues are about reliability. They are not security issues in and of themselves.
If the DoS leads to some other other issues like memory corruption or getting access to information that shouldn't be available, then it would track as security.
JSON parsing should detect embedded \0 values