Bounds-checks issues with Fast FP parser (placeholder)
cowtowncoder opened this issue · 4 comments
(note: this is a placeholder until individual issues are created)
It looks like the new "fast FP parsing" functionality has a few issues uncoved by OssFuzz (https://oss-fuzz.com/) jackson-core project. This is not unsurprising as fuzzers are good at finding various edge cases for invalid input.
I will need to figure out how to give access to actual reports; in the meantime, this issue can act as a placeholder for various instances.
@pjfanning I am hoping to give you access to OssFuzz jackson projects -- I don't maintain them (nor written code) but collaborate with author(s). Full information is not by default world-accessible, due to possible security issues as some problems could be considered Vulnerabilities.
The first issue (I think there are more, but first I saw) is reported for invalid input, with stack trace of:
== Java Exception: java.lang.StringIndexOutOfBoundsException: String index out of range: 3
--
| at java.base/java.lang.StringLatin1.charAt(StringLatin1.java:48)
| at java.base/java.lang.String.charAt(String.java:712)
| at com.fasterxml.jackson.core.io.doubleparser.AbstractFloatingPointBitsFromCharSequence.parseHexFloatLiteral(AbstractFloatingPointBitsFromCharSequence.java:331)
| at com.fasterxml.jackson.core.io.doubleparser.AbstractFloatingPointBitsFromCharSequence.parseFloatingPointLiteral(AbstractFloatingPointBitsFromCharSequence.java:215)
and oss-fuzz testcase has actual input data (which we'll need to repro and perhaps request a fix or do the fix).
It seems like a missing bounds-check of some kind; I have fixed similar ones from jackson-core multiple times.
@wrandelshofer AbstractFloatingPointBitsFromCharSequence is copied from your FastDoubleParser project. I don't yet have the input that caused the problem but I just thought I'd make you aware that there may be an issue.
@cowtowncoder Jackson doesn't really support hex numbers anyway so it is unlikely that Jackson parsers would run into this issue. Users could use NumberInput class directly but this not really an encouraged use case. I'd still like to get this fixed - I'm just highlighting that this is relatively low impact for Jackson users.
@pjfanning I could be wrong but I don't think Fuzzer calls these methods directly. So it would seem like code somehow managed to trigger execution. Most likely it'd be root-level JSON value that starts with something like 1.0 or -0. that would trigger code path in question.
I 100% agree that if this is not the case (and Fuzzer called helper methods directly), we really shouldn't care much.
It is odd tho.. how Hex-value case would get that far. JsonParser does, I think, check lexical validity of Double numbers. And hex-values do not match that.
So I guess figuring out the specific Fuzz code, input, is important here.
Fuzzer input file looks like garbage, but I suspect it might be due to encoding actual settings as prefix. Or something.
Regardless, PR merged should resolve it; closing this.