Cloning a `Pin<Box>` with custom allocator can violate the drop guarantee without ever calling `Box::pin` or similar

[maxdexh]

Like #157089, this uses a custom clone impl on Box<LocalType, A> to trick the derive(Clone) on Pin into pinning a box with an inelligible allocator. However, unlike that issue, this one creates the initial Pin<Box> using a type that implements Unpin, thus bypassing all current attempts to fix the issue through stricter requirements on Box -> Pin<Box> conversions. There is also no variance involved here, only unsize coercions through Pin<Box>.

playground (using a wrapper for bumpalo due to missing feature flags)

#![feature(allocator_api)]

use std::{alloc::Allocator, marker::PhantomPinned, pin::Pin};

use bumpalo::Bump;

// This type is `Unpin`
type BoxParts<T, A> = (fn() -> T, A);

// Goal: Use a `Clone` impl on `Box<dyn Helper<T, A>, A>` to turn `Box<BoxParts<T, A>, A>` into `Box<T,A>`, 
// so that `Pin::clone` automatically pins it in an arbitrary allocator, which is unsound.
trait Helper<T, A> {
    fn parts(&self) -> BoxParts<T, A>;
    fn downcast(self: Pin<&mut Self>) -> Pin<&mut T>;
}
impl<T, A: Clone> Helper<T, A> for BoxParts<T, A> {
    fn parts(&self) -> Self {
        self.clone()
    }
    fn downcast(self: Pin<&mut Self>) -> Pin<&mut T> {
        unreachable!("never called")
    }
}
impl<T, A> Helper<T, A> for T {
    fn parts(&self) -> BoxParts<T, A> {
        unreachable!("never called")
    }
    fn downcast(self: Pin<&mut Self>) -> Pin<&mut T> {
        self
    }
}
impl<'a, T: 'a, A: Allocator> Clone for Box<dyn Helper<T, A> + 'a, A> {
    fn clone(&self) -> Self {
        let (fac, alloc) = (**self).parts();
        Box::new_in(fac(), alloc)
    }
}

fn break_drop_guarantee<T>(fac: fn() -> T) {
    let mut short = Bump::new();

    // no requirements for the allocator, we are just pinning a `Box<impl Unpin>` 
    let base: Pin<Box<BoxParts<T, &Bump>, &Bump>> = Pin::new(Box::new_in((fac, &short), &short));

    // unsize coercion
    let base: Pin<Box<dyn Helper<T, &Bump> + '_, &Bump>> = base;

    // Make `Clone` produce a different value to bypass the Box::pin requirements
    let mut bad: Pin<Box<dyn Helper<T, &Bump> + '_, &Bump>> = base.clone();

    // the destructor of `T` must run before its memory is reused
    let proof: Pin<&mut T> = bad.as_mut().downcast();

    // payload: 0x57dcbd949390
    println!("payload: {:p}", proof);

    // get rid of the borrows of `short`
    std::mem::forget(base);
    std::mem::forget(bad);

    // deallocate `T` without running its destructor
    short.reset();

    // unsoundly reuse the memory of `T`
    let reuse = short.alloc([0u8; 1024]);

    // reuse: 0x57dcbd949390
    println!("reuse: {:p}", reuse);
}

pub fn main() {
    struct Victim(#[expect(dead_code)] [u8; 1024], PhantomPinned);
    impl Drop for Victim {
        fn drop(&mut self) {
            unreachable!("never run")
        }
    }
    break_drop_guarantee(|| Victim([0; _], PhantomPinned));
}

[maxdexh]

@rustbot label +A-allocators A-box A-pin requires-nightly

[maxdexh]

@rustbot label T-types T-libs-api I-unsound

[nia-e]

i think i have a migraine.

all other points aside, doesn't this mean you can already launder arbitrary maybe-not-unpinnable pointers into a Pin? that seems like a bigger issue if so :D

[maxdexh]

Nope, I think it just happens to work out without allocators. This issue uses three things: the Deref impl of Box, the fundamentalness of Box and the unsize coercibility of Box. Removing any of these (even if only for custom allocators) fixes the issue.

[maxdexh]

I will agree though, this one is a headscratcher. I am surprised that theemathas didn't find this first because the other issue was found after they were discussing unsize coercions.

[maxdexh]

This issue uses three things: the Deref impl of Box, the fundamentalness of Box and the unsize coercibility of Box. Removing any of these (even if only for custom allocators) fixes the issue.

The nuclear option would be to just throw out the Allocator param of Box and make a new type for that. More optimistically, I hope this issue can just be fixed by restricting #[fundamental] Box to only work for the Global allocator? Because all the other options I can think of seem unpalatable.