liebharc / basic_dsp

Basic DSP vector operations for Rust.
Apache License 2.0
43 stars 5 forks source link

panic safety issue in `impl TransformContent<S, D> for [S; (2|3|4)]` #47

Closed JOE1994 closed 3 years ago

JOE1994 commented 3 years ago

Hello :crab: , we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

The issue is relevant to implementation of TransformContent<S, D> trait for [S; 2], [S; 3], and [S; 4].

https://github.com/liebharc/basic_dsp/blob/7375e9f02769c95bf1bdb58cb2130afcccdf3f50/matrix/src/lib.rs#L229-L241

https://github.com/liebharc/basic_dsp/blob/7375e9f02769c95bf1bdb58cb2130afcccdf3f50/matrix/src/lib.rs#L243-L258

If a panic happens within conversion, item(S) within self can be dropped twice since the ownership of the item within self is duplicated with ptr::read().

Suggested Fix

By keeping self within ManuallyDrop<_> instead of using mem::forget(), it is possible to guard against such double drop bugs. I will immediately submit a PR containing the suggested fix.

Thank you for checking out this issue :+1: