Merge pull request #19 from steffahn/prevent_unsound_field_swapping

Prevent unsound field swapping

.github/workflows/rust.yml

@@ -1,6 +1,6 @@
 name: Rust
 
-on: [push]
+on: [push, pull_request]
 
 jobs:
   native:

src/lib.rs

@@ -310,6 +310,7 @@ macro_rules! self_cell {
     $(#[$StructMeta])*
     $Vis struct $StructName $(<$OwnerLifetime>)* {
         unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell<
+            $StructName$(<$OwnerLifetime>)?,
             $Owner,
             $Dependent<'static>
         >,
@@ -509,6 +510,7 @@ macro_rules! self_cell {
             let unsafe_self_cell = unsafe { core::mem::transmute::<
                 Self,
                 $crate::unsafe_self_cell::UnsafeSelfCell<
+                    $StructName$(<$OwnerLifetime>)?,
                     $Owner,
                     $Dependent<'static>
                 >

src/unsafe_self_cell.rs

@@ -27,18 +27,24 @@ pub struct JoinedCell<Owner, Dependent> {
 // Library controlled struct that marks all accesses as unsafe.
 // Because the macro generated struct impl can be extended, could be unsafe.
 #[doc(hidden)]
-pub struct UnsafeSelfCell<Owner, DependentStatic: 'static> {
+pub struct UnsafeSelfCell<ContainedIn, Owner, DependentStatic: 'static> {
     joined_void_ptr: NonNull<u8>,
 
+    // ContainedIn is necessary for type safety since we don't fully
+    // prohibit access to the UnsafeSelfCell; swapping between different
+    // structs can be unsafe otherwise, see Issue #17.
+    contained_in_marker: PhantomData<ContainedIn>,
+
     owner_marker: PhantomData<Owner>,
     // DependentStatic is only used to correctly derive Send and Sync.
     dependent_marker: PhantomData<DependentStatic>,
 }
 
-impl<Owner, DependentStatic> UnsafeSelfCell<Owner, DependentStatic> {
+impl<ContainedIn, Owner, DependentStatic> UnsafeSelfCell<ContainedIn, Owner, DependentStatic> {
     pub unsafe fn new(joined_void_ptr: NonNull<u8>) -> Self {
         Self {
             joined_void_ptr,
+            contained_in_marker: PhantomData,
             owner_marker: PhantomData,
             dependent_marker: PhantomData,
         }
@@ -96,15 +102,17 @@ impl<Owner, DependentStatic> UnsafeSelfCell<Owner, DependentStatic> {
     }
 }
 
-unsafe impl<Owner, DependentStatic> Send for UnsafeSelfCell<Owner, DependentStatic>
+unsafe impl<ContainedIn, Owner, DependentStatic> Send
+    for UnsafeSelfCell<ContainedIn, Owner, DependentStatic>
 where
     // Only derive Send if Owner and DependentStatic is also Send
     Owner: Send,
     DependentStatic: Send,
 {
 }
 
-unsafe impl<Owner, DependentStatic> Sync for UnsafeSelfCell<Owner, DependentStatic>
+unsafe impl<ContainedIn, Owner, DependentStatic> Sync
+    for UnsafeSelfCell<ContainedIn, Owner, DependentStatic>
 where
     // Only derive Sync if Owner and DependentStatic is also Sync
     Owner: Sync,

tests/invalid/swap_cell_member.rs

@@ -0,0 +1,37 @@
+use self_cell::self_cell;
+
+type Dep1<'a> = (&'a str, &'static str);
+
+self_cell! {
+    pub struct Struct1 {
+        owner: String,
+        #[covariant]
+        dependent: Dep1,
+    }
+}
+
+type Dep2<'a> = (&'static str, &'a str);
+
+self_cell! {
+    pub struct Struct2 {
+        owner: String,
+        #[covariant]
+        dependent: Dep2,
+    }
+}
+
+fn main() {
+    let hello: &'static str;
+    {
+        let mut x1 = Struct1::new(String::from("Hello World"), |s| (s, ""));
+        let mut x2 = Struct2::new(String::new(), |_| ("", ""));
+        std::mem::swap(&mut x1.unsafe_self_cell, &mut x2.unsafe_self_cell);
+        hello = x2.borrow_dependent().0;
+
+        dbg!(hello); // "Hello World"
+                     // hello is now a static reference in to the "Hello World" string
+    }
+    // the String is dropped at the end of the block above
+
+    dbg!(hello); // prints garbage, use-after-free
+}

tests/invalid/swap_cell_member.stderr

@@ -0,0 +1,8 @@
+error[E0308]: mismatched types
+  --> $DIR/swap_cell_member.rs:28:50
+   |
+28 |         std::mem::swap(&mut x1.unsafe_self_cell, &mut x2.unsafe_self_cell);
+   |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `Struct1`, found struct `Struct2`
+   |
+   = note: expected mutable reference `&mut UnsafeSelfCell<Struct1, String, (&'static str, &'static str)>`
+              found mutable reference `&mut UnsafeSelfCell<Struct2, String, (&'static str, &'static str)>`