Fuzz: serialization heap buffer overflow via round-trip and deserialize

[uniphil]

Cardinality estimator makes assumptions about the values it receives when deserializing which seem to be upheld by serde_json but not serde in general, leading to buffer overflows and other problems when used with other serde de/serializers.

I had the wildest crashes happening in a project, which I traced to cardinality-estimator. In narrowing down a smaller reproducible example, I found that using bincode with this library was sufficient to get buffer overflows: see https://github.com/uniphil/whose-overflow-is-it-anyway

This PR adds a fuzz target for postcard, whose API is close enough to serde_json for the change from the serde fuzz target to be a single line. It also adds postcard calls to the normal serde tests, though I wasn't able to specifically write a failing regression test case for it (I'm sure it's possible, just had to stop somewhere).

To be clear: this change only illustrates the issue and does not include a fix. Seems like fixing will require either

• fixing the deserialization logic to assume less about the inputs it receives (array alignment and length?), OR
• documenting explicitly that the serde support is only safe to work with serde_json.

Thanks @DavidBuchanan314 for help narrowing the source of the crashes <3

[austinhartzheim]

Thanks for the write-up and fuzzing harness! I'll try to take a look in the next few days.

[uniphil]

Thanks for the quick reply!

I added another fuzz harness for serde_json that directly creates arrays with serde_json::Value, and unfortunately this also quickly discovers the heap buffer overflow. So I think it might be a deserialization problem in general, and not one necessarily hidden by serde_json.

[austinhartzheim]

Thank you. I did look at this last night and did notice that it is reproducible with serde_json.

I have a working patch, but want to spend a bit more time making sure it's property tested.

[uniphil]

amazing! looking forward to it :)

[uniphil]

Hey @austinhartzheim just a tiny nudge: any chance you can share the patch, if testing/getting it ready to merge will take more time?

I'll find another workaround if not, just checking

[uniphil]

fyi: i added another fuzz harness that shows the heap buffer overflow even without malicious bytes being fed to the deserializer: all fuzz inputs are only .inserted to the estimator, but serializing and then deserializing still reproduces the problem.