Re: [I] Soundness Bug in `try_binary` when `Array` is implemented incorrectly in external crate [arrow-rs]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on issue #9106:
URL: https://github.com/apache/arrow-rs/issues/9106#issuecomment-3719834544

   Thank you for this report @shinmao . I wanted to provide some background context and analysis of the impact of this issue. 
   
   # Analysis
   
   After consideration, we agree this is technically a soundness bug. However, since the `Array` trait is designed to implement the Apache Arrow specification, we know of no legitimate use case for implementing `Array` for types outside the spec (although @gabotechs mentions he had done it here https://github.com/apache/arrow-rs/pull/9092#issuecomment-3710695154) and overriding the `Array` trait in this way is not a common or expected use of this library. 
   
   We think the impact of this issue is minimal as it isn't something users are likely to run into (there are no widely used crates that implement the `Array` trait) and this this is not likely to produce an exploit chain. 
   
   Thus our collective opinion is that it is most appropriate to treat it as a normal bug. 
   
   # Remediation
   
   To prevent such issues we have chosen to seal the `Array` trait to prevent third-party implementation (@tustvold  did so in https://github.com/apache/arrow-rs/pull/9092)
   
   # Potential RUSTSEC advisory
   
   Given the limited impact of this bug, I don't think it warrants a rustsec advisory as I don't think this is an exploitable error for users of arrow-rs. 
   
   If we found a downstream crate that had overridden Array in such a way that would allow users to trigger undefined behavior, I think we would treat it as a bug (and a rustsec advisory) for that downstream crate.
   
   However, there may be different opinions about this, and we would welcome hearing them. 
   
   If we are to issue a RUSTSEC advisory, given a look at the dependencies from crates.io
   - https://crates.io/crates/arrow/reverse_dependencies
   
   I think we would want to make patch releases for at least arrow 56.x line and possible also the arrow 55.x line to help others


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org