Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent abuse of private module #336

Merged
merged 1 commit into from
Dec 26, 2021
Merged

Prevent abuse of private module #336

merged 1 commit into from
Dec 26, 2021

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Dec 26, 2021

This was originally reported by @danielhenrymantilla in https://discord.com/channels/273534239310479360/512792629516173323/870075511009857617.

Currently, you can break the soundness of the pin-project by overriding the private module as follows.
Considering that it uses a hidden private API that is not guaranteed to be stable, I don't think this will actually happen except in cases where users intentionally abuse it or use a malicious crate. Also, in such a case, fixing this would not help much because the attacker can do anything.
That said, it seems relatively easy to fix this, so I'm going to fix this.

extern crate pin_project as pin_project_orig;
extern crate self as pin_project;

pub use ::pin_project_orig::*;
mod __private {
    pub use ::pin_project_orig::__private::*;
    pub trait Drop {}
}

use std::{marker::PhantomPinned, mem};

#[pin_project]
struct S {
    #[pin]
    f: (u8, PhantomPinned),
}

impl Drop for S {
    fn drop(&mut self) {
        let prev = &self.f.0 as *const _ as usize;
        let moved = mem::take(&mut self.f); // move pinned field
        let moved = &moved.0 as *const _ as usize;
        assert_eq!(prev, moved); // panic
    }
}

fn main() {
    let mut x = Box::pin(S { f: (1, PhantomPinned) });
    let _f = x.as_mut().project().f; // first mutable access
}

playground


This patch forces the macro to refer to the pin-project crate by referring to the _pin_project imported into the const _ scope instead of ::pin_project.

const _: () = {
#[allow(unused_extern_crates)]
extern crate pin_project as _pin_project;

pinned: _pin_project::__private::Pin::new_unchecked(pinned),

In the above example, a user-implemented Drop is detected, resulting in an error.

error[E0119]: conflicting implementations of trait `_::SMustNotImplDrop` for type `S`
--> tests/ui/pin_project/override-priv-mod.rs:14:1
|
14 | #[pin_project] //~ ERROR conflicting implementations of trait `_::FooMustNotImplDrop`
| ^^^^^^^^^^^^^^
| |
| first implementation here
| conflicting implementation for `S`
|
= note: this error originates in the derive macro `::pin_project::__private::__PinProjectInternalDerive` (in Nightly builds, run with -Z macro-backtrace for more info)

If other items (e.g., PinnedDrop) are replaced in the same way, either nothing happens (nothing is affected because macro doesn't use them), or #[pin_project] and #[pinned_drop] refer to different items, resulting in an error.

@taiki-e
Copy link
Owner Author

taiki-e commented Dec 26, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 26, 2021

Build succeeded:

@bors bors bot merged commit 27c1aa2 into main Dec 26, 2021
@bors bors bot deleted the priv-mod branch December 26, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant