From 1a4c11a675327ea94b78de6a571b343892cb2ab8 Mon Sep 17 00:00:00 2001
From: Christopher Durham <cad97@cad97.com>
Date: Fri, 23 Jul 2021 00:12:56 -0500
Subject: [PATCH] Replace unsound usage of transmute_copy (#72)

* Use transmute! to enforce safer transmute_copy use

* Test fix for #71
---
 src/contiguous.rs        |  9 ++++-----
 src/lib.rs               | 11 +++++++++--
 src/transparent.rs       | 12 ++++++------
 tests/wrapper_forgets.rs | 13 +++++++++++++
 4 files changed, 32 insertions(+), 13 deletions(-)
 create mode 100644 tests/wrapper_forgets.rs

diff --git a/src/contiguous.rs b/src/contiguous.rs
index b60f4a4..538514b 100644
--- a/src/contiguous.rs
+++ b/src/contiguous.rs
@@ -1,5 +1,4 @@
 use super::*;
-use core::mem::{size_of, transmute_copy};
 
 /// A trait indicating that:
 ///
@@ -127,10 +126,10 @@ pub unsafe trait Contiguous: Copy + 'static {
       // they've sworn under the Oath Of Unsafe Rust that that already
       // matched) so this is allowed by `Contiguous`'s unsafe contract.
       //
-      // So, the `transmute_copy`. ideally we'd use transmute here, which
+      // So, the `transmute!`. ideally we'd use transmute here, which
       // is more obviously safe. Sadly, we can't, as these types still
       // have unspecified sizes.
-      Some(unsafe { transmute_copy::<Self::Int, Self>(&value) })
+      Some(unsafe { transmute!(value) })
     } else {
       None
     }
@@ -161,9 +160,9 @@ pub unsafe trait Contiguous: Copy + 'static {
 
     // SAFETY: The unsafe contract requires that these have identical
     // representations, and that the range be entirely valid. Using
-    // transmute_copy instead of transmute here is annoying, but is required
+    // transmute! instead of transmute here is annoying, but is required
     // as `Self` and `Self::Int` have unspecified sizes still.
-    unsafe { transmute_copy::<Self, Self::Int>(&self) }
+    unsafe { transmute!(self) }
   }
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index 89b4d5f..06fd5ea 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -57,6 +57,13 @@ macro_rules! impl_unsafe_marker_for_array {
   }
 }
 
+/// A macro to transmute between two types without requiring knowing size statically.
+macro_rules! transmute {
+  ($val:expr) => {
+    transmute_copy(&ManuallyDrop::new($val))
+  };
+}
+
 #[cfg(feature = "extern_crate_std")]
 extern crate std;
 
@@ -248,7 +255,7 @@ impl std::error::Error for PodCastError {}
 #[inline]
 pub fn cast<A: Pod, B: Pod>(a: A) -> B {
   if size_of::<A>() == size_of::<B>() {
-    unsafe { core::mem::transmute_copy(&a) }
+    unsafe { transmute!(a) }
   } else {
     something_went_wrong("cast", PodCastError::SizeMismatch)
   }
@@ -349,7 +356,7 @@ pub fn pod_align_to_mut<T: Pod, U: Pod>(
 #[inline]
 pub fn try_cast<A: Pod, B: Pod>(a: A) -> Result<B, PodCastError> {
   if size_of::<A>() == size_of::<B>() {
-    Ok(unsafe { core::mem::transmute_copy(&a) })
+    Ok(unsafe { transmute!(a) })
   } else {
     Err(PodCastError::SizeMismatch)
   }
diff --git a/src/transparent.rs b/src/transparent.rs
index fb1dddf..0329d1c 100644
--- a/src/transparent.rs
+++ b/src/transparent.rs
@@ -94,7 +94,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
   {
     // SAFETY: The unsafe contract requires that `Self` and `Inner` have
     // identical representations.
-    unsafe { transmute_copy(&s) }
+    unsafe { transmute!(s) }
   }
 
   /// Convert a reference to the inner type into a reference to the wrapper
@@ -110,7 +110,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
       // SAFETY: The unsafe contract requires that these two have
       // identical representations.
       let inner_ptr = s as *const Inner;
-      let wrapper_ptr: *const Self = transmute_copy(&inner_ptr);
+      let wrapper_ptr: *const Self = transmute!(inner_ptr);
       &*wrapper_ptr
     }
   }
@@ -128,7 +128,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
       // SAFETY: The unsafe contract requires that these two have
       // identical representations.
       let inner_ptr = s as *mut Inner;
-      let wrapper_ptr: *mut Self = transmute_copy(&inner_ptr);
+      let wrapper_ptr: *mut Self = transmute!(inner_ptr);
       &mut *wrapper_ptr
     }
   }
@@ -173,7 +173,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
     Self: Sized,
     Inner: Sized,
   {
-    unsafe { transmute_copy(&s) }
+    unsafe { transmute!(s) }
   }
 
   /// Convert a reference to the wrapper type into a reference to the inner
@@ -189,7 +189,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
       // SAFETY: The unsafe contract requires that these two have
       // identical representations.
       let wrapper_ptr = s as *const Self;
-      let inner_ptr: *const Inner = transmute_copy(&wrapper_ptr);
+      let inner_ptr: *const Inner = transmute!(wrapper_ptr);
       &*inner_ptr
     }
   }
@@ -207,7 +207,7 @@ pub unsafe trait TransparentWrapper<Inner: ?Sized> {
       // SAFETY: The unsafe contract requires that these two have
       // identical representations.
       let wrapper_ptr = s as *mut Self;
-      let inner_ptr: *mut Inner = transmute_copy(&wrapper_ptr);
+      let inner_ptr: *mut Inner = transmute!(wrapper_ptr);
       &mut *inner_ptr
     }
   }
diff --git a/tests/wrapper_forgets.rs b/tests/wrapper_forgets.rs
new file mode 100644
index 0000000..5484744
--- /dev/null
+++ b/tests/wrapper_forgets.rs
@@ -0,0 +1,13 @@
+use bytemuck::TransparentWrapper;
+
+#[repr(transparent)]
+struct Wrap(Box<u32>);
+
+// SAFETY: it's #[repr(transparent)]
+unsafe impl TransparentWrapper<Box<u32>> for Wrap {}
+
+fn main() {
+  let value = Box::new(5);
+  // This used to duplicate the wrapped value, creating a double free :(
+  Wrap::wrap(value);
+}