search

outfoxx / PotentCodables

🧪 PotentCodables - A potent set of implementations and extensions to the Swift Codable system
MIT License
69 stars 12 forks source link

CBOR serialisation crashes #65

Open marius-se opened 8 months ago

marius-se commented 8 months ago

Similar to SwiftCBOR trying to serialise a lot of invalid CBOR data crashes the entire app. Here is a small reproducible project:

potent_cbor_crash.zip

I saw that the Go CBOR library has different serialisation modes (one specifically for WebAuthn, which is the target application I'm working on) and a "wellformed" check (which IIRC is optional), maybe we could implement something similar for PotentCBOR to work around this crash?

kdubb commented 8 months ago

I'm very much in support of doing all we can to ensure ill-formed data is reported but doesn't crash the app.

I assume throwing an error is fine?

Thanks for reproducer!

marius-se commented 8 months ago

Cool! Yeah throwing an error should be fine! Adding the wellformed checks will likely decrease performance, so maybe we should make it optional?

kdubb commented 8 months ago

I'm not sure all the checks that Go CBOR does but I would say safety is a concern and CBOR is generally used for the externally sourced data (e.g. network, etc.)... disabling safety for a small performance gain is not really something that should be done.

I ran your reproducer and for that case it seems to simply need a loop counter and to check it against a maximum on every nested call. I can't imagine that disabling that would show any performance advantage.

marius-se commented 8 months ago

Oh okay! I don't really know how CBOR or the safety checks work under the hood, but if it's really just a nested level check you're right and it shouldn't kill performance!