FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
316 stars 136 forks source link

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

Closed arthurscchan closed 11 months ago

arthurscchan commented 11 months ago

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.

cowtowncoder commented 11 months ago

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 commented 11 months ago

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.

arthurscchan commented 11 months ago

@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 commented 11 months ago

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