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

Fixes for #426: Add bound check for array index #427

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

arthurscchan
Copy link
Contributor

This PR provides a suggested fix for #426 by adding a bound check before the ptr Integer used as the index for accessing the _inputBuffer array.

Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 19, 2023

Ok, so I think test is useful but I don't like the fix here -- it works around the problem but doesn't actually solve the problem.

Would it be possible to just add failing test (under src/test/java/..../failing) int this PR and I can see what goes wrong with decoding wrt sequence of calls.

EDIT: I'll just get code from this branch and run the test; should be fine.

@cowtowncoder cowtowncoder added ion 2.17 smile fuzz Issue found by OssFuzz and removed ion labels Dec 19, 2023
@cowtowncoder
Copy link
Member

Interesting. The token in the beginning if JsonToken.VALUE_EMBEDDED_OBJECT representing binary data.
It's invalid, specifies huge length; but skipping produces invalid state.

@cowtowncoder cowtowncoder added this to the 2.17.0 milestone Dec 19, 2023
@cowtowncoder cowtowncoder merged commit 7e69549 into FasterXML:2.17 Dec 19, 2023
4 checks passed
@arthurscchan
Copy link
Contributor Author

@cowtowncoder Thanks for merging them in. I just had the chance to see your comments and you already fixed that for me. Thanks for your prompt reply. Seems that it justifies the use of fuzzers, especially for these large, strange and malformed inputs.

@cowtowncoder
Copy link
Member

Yes, I think fuzzing is good as long as we can replicate their findings. Picks up many edge cases.

@arthurscchan arthurscchan deleted the 426-fix branch January 17, 2024 04:56
@arthurscchan arthurscchan restored the 426-fix branch January 17, 2024 04:56
@arthurscchan arthurscchan deleted the 426-fix branch January 17, 2024 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 fuzz Issue found by OssFuzz smile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants