double drop can happen upon panic within user provided impl of `Endian` trait
Hello
Issue Description
If a user-provided implementation of Endian
panics, a double-drop error happens since the ownership of an object is
unsafely duplicated by ptr::read(elt)
.
https://gitlab.com/myrrlyn/endian_trait/-/blob/master/src/slices.rs#L7-33
/// Traverse a slice, performing the `Endian` method on each item in place.
impl<'a, T: Endian> Endian for &'a mut [T] {
fn from_be(self) -> Self {
for elt in self.iter_mut() { unsafe {
ptr::write(elt, ptr::read(elt).from_be());
} }
self
}
fn from_le(self) -> Self {
for elt in self.iter_mut() { unsafe {
ptr::write(elt, ptr::read(elt).from_le());
} }
self
}
fn to_be(self) -> Self {
for elt in self.iter_mut() { unsafe {
ptr::write(elt, ptr::read(elt).to_be());
} }
self
}
fn to_le(self) -> Self {
for elt in self.iter_mut() { unsafe {
ptr::write(elt, ptr::read(elt).to_le());
} }
self
}
}
Proof of Concept
Below example program exhibits heap corruption upon a panic within the user-provided Endian
implementation.
use endian_trait::Endian;
#[derive(Debug)]
struct Foo(Box<Option<i32>>);
impl Endian for Foo {
fn to_be(self) -> Self {
println!(
"PANIC BY MISTAKE {}",
self.0.as_ref().as_ref().unwrap()
);
self
}
fn to_le(self) -> Self { self }
fn from_be(self) -> Self { self }
fn from_le(self) -> Self { self }
}
fn main() {
let mut foo = [Foo(Box::new(None))];
let x = (&mut foo).to_be();
dbg!(x);
}
When the below program is executed on Windows 10, the output was as below
error: process didn't exit successfully: `target\debug\garbage.exe` (exit code: 0xc0000374, STATUS_HEAP_CORRUPTION)
Suggested Fix
The fact that the APIs defined on Endian
trait take owned self
as input
makes it quite difficult to come up with a neat code fix for this issue.
A solution I can think of is as below.
- Make the
Endian
trait anunsafe
trait. - Add a warning to the documentation about implementing the
Endian
trait: users are responsible to make sure no panics happen within the implementation ofEndian
.
Thank you for checking out this issue