-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
We recently had a security report filed here
The core issue is that there are places in the arrow-rs codebase that rely on the results returned from the Array trait to be consistent, otherwise undefined behavior can result
Our fix was to make the Array trait as sealed (which means it can not be implemented outside the arrow-rs crate).
While this certainly avoids the undefined behavior, it is a heavy hammer. Our initial assumption that implementing the Array trait was not common / good for other crates to extend the Array trait. However, we now have at least two reports that it was done:
Describe the solution you'd like
Given that there are (seemingly) a non trivial number of uses of Array, I think it is worth reconsidering the change and finding some other way to resolve the soundness bug.
For example, we could unseal Array
Describe alternatives you've considered
One thing we could potentially do is mark the Array trait as unsafe: https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#implementing-an-unsafe-trait
That way people could still implement Array, but would have to explicitly acknowledge that there is danger in doing so
Additional context