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

OpaqueTypedefUnsized::from_* are unsound #1

Closed
dtolnay opened this Issue Dec 5, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@dtolnay
Copy link

dtolnay commented Dec 5, 2017

This example from the readme may work on the current version of Rust on our current targets but is undefined behavior. The compiler is free to represent MyStr different from str and that would lead to memory unsafety. If you need this to be well-defined, your macro needs to check for a #[repr(C)] on the newtype struct and panic if there is not one, similar to ref-cast.

#[macro_use]
extern crate opaque_typedef_macros;
extern crate opaque_typedef;

use opaque_typedef::OpaqueTypedefUnsized;

#[derive(Debug, OpaqueTypedefUnsized)]
struct MyStr(str);

fn main() {
    println!("{:?}", MyStr::from_inner("s").unwrap());
}

@lo48576 lo48576 added the bug label Dec 6, 2017

@lo48576

This comment has been minimized.

Copy link
Owner

lo48576 commented Dec 6, 2017

Thank you for reporting.

I implemented that part (from_inner*) by referring to libstd and rocket crate (especially parts below).

std::ffi::OsStr (in rust-1.22.1):
https://github.com/rust-lang/rust/blob/05e2e1c41414e8fc73d0f267ea8dab1a3eeeaa99/src/libstd/ffi/os_str.rs#L44-L50
https://github.com/rust-lang/rust/blob/05e2e1c41414e8fc73d0f267ea8dab1a3eeeaa99/src/libstd/ffi/os_str.rs#L396-L398

std::sys::unix::os_str::Slice (in rust-1.22.1):
https://github.com/rust-lang/rust/blob/05e2e1c41414e8fc73d0f267ea8dab1a3eeeaa99/src/libstd/sys/unix/os_str.rs#L26-L28
https://github.com/rust-lang/rust/blob/05e2e1c41414e8fc73d0f267ea8dab1a3eeeaa99/src/libstd/sys/unix/os_str.rs#L128-L135

rocket::http::uncased::UncasedStr (in rocket-0.3.2):
https://github.com/SergioBenitez/Rocket/blob/5dabb00f2da310e3fe1400517558f263b276614d/lib/src/http/uncased.rs#L22-L41

They use unsafe { mem::transmute(s) } or unsafe { &*(string as *const str as *const UncasedStr) }, but their outer type (OsStr, Slice and UncasedStr) doesn't specify #[repr(*)].
Do they have the same soundness issue as my crate, or do they have special trick (like specific target triple or something)?

@dtolnay

This comment has been minimized.

Copy link

dtolnay commented Dec 6, 2017

Generally unsafe transmutes in the standard library are not safe to copy into your code. If the compiler ever makes a change that would break those transmutes, they would just fix the transmutes in the same release. Whereas in your code such a change would result in unsoundness.

The Rocket example is undefined behavior.

@lo48576

This comment has been minimized.

Copy link
Owner

lo48576 commented Dec 6, 2017

I understand.
I'll fix this later.
Thank you.

lo48576 added a commit that referenced this issue Dec 9, 2017

@lo48576

This comment has been minimized.

Copy link
Owner

lo48576 commented Dec 9, 2017

Fixed.

@lo48576

This comment has been minimized.

Copy link
Owner

lo48576 commented Aug 2, 2018

https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1280-2018-08-02

The #[repr(transparent)] attribute is now stable. This attribute allows a Rust newtype wrapper (struct NewType<T>(T);) to be represented as the inner type across Foreign Function Interface (FFI) boundaries.

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment