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

Bounds-checks issues with Fast FP parser (placeholder) #809

Closed
cowtowncoder opened this issue Aug 12, 2022 · 4 comments
Closed

Bounds-checks issues with Fast FP parser (placeholder) #809

cowtowncoder opened this issue Aug 12, 2022 · 4 comments
Labels
2.14 Issue planned (at earliest) for 2.14 oss-fuzz Issue uncovered by oss-fuzz fuzzer

Comments

@cowtowncoder
Copy link
Member

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

@cowtowncoder cowtowncoder added the 2.14 Issue planned (at earliest) for 2.14 label Aug 12, 2022
@cowtowncoder
Copy link
Member Author

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

@cowtowncoder cowtowncoder added the oss-fuzz Issue uncovered by oss-fuzz fuzzer label Aug 12, 2022
@pjfanning
Copy link
Member

pjfanning commented Aug 12, 2022

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

@cowtowncoder
Copy link
Member Author

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

@cowtowncoder
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.14 Issue planned (at earliest) for 2.14 oss-fuzz Issue uncovered by oss-fuzz fuzzer
Projects
None yet
Development

No branches or pull requests

2 participants