near / borsh-rs

Rust implementation of Binary Object Representation Serializer for Hashing
https://borsh.io/
Apache License 2.0
306 stars 67 forks source link

Can't serialize a `Vec<ZST>`, but it appears that the attack is only on deserialization #301

Open richardpringle opened 3 months ago

richardpringle commented 3 months ago

I understand the issue with deserializing a Vec<ZST>, (#52 and #19), but if I'm not mistaken, there is no such attack on the serialization side.

It seems unnecessary to do the ZST check for serialization. If it is unnecessary, what are the chances you could remove that check? If it is necessary, could you point me to the documentation as to why? Or maybe could you document why, if it isn't already?

This just seems wrong to me:

#[test]
fn serialize_zst() {
    let xs = borsh::to_vec(&vec![(); 32]).unwrap();
}
called `Result::unwrap()` on an `Err` value: Custom { kind: InvalidData, error: "Collections of zero-sized types are not allowed due to deny-of-service concerns on deserialization." }

Specifically the

due to deny-of-service concerns on deserialization.

I'm not deserializing, the check already exists on deserialization, preventing the attack.

dj8yfo commented 3 months ago

@richardpringle

Or maybe could you document why,

to notify early that deserialization won't be possible . We want to primarily support A -> serialized form -> A cases, not variations of arbitrary B -> serialized form -> C. A notable exception which was done in the lib is support of some cases of A -> serialized form -> B, where B: Borrow<A>. If you feel like contributing a short section to doc anywhere you like, it would be most welcome.