Confusion between \0 and EOF can lead to OutOfMemoryError

[eamonnmcmanus]

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.

[johnjaylward]

Do you have an actual test to validate this?

I'd argue that valid text input should never have a \0 in 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 JSONException

[eamonnmcmanus]

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, \0 should mean EOF everywhere, but it currently doesn't.

[johnjaylward]

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]

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.)

[stleary]

@eamonnmcmanus Good catch and thanks for bringing this up. In this case, it's OK to post the recreate code and proposed fix.

[johnjaylward]

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.