Derive macros are unsound due to types being mentioned twice
- Dominant language
- Rust
- Stars
- 2.6k
- Forks
- 179
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 29
Description
This is a different unsoundness than https://github.com/google/zerocopy/issues/388.
The derive macros expand to code that mentions the field types, in order to validate that they implement the expected traits. This doesn't work if mentioning the same type name twice produces a different type. This can happen if the type is defined as a macro invocation that's nondeterministic.
The code below causes a segmentation fault. (Also [in a zip file](https://github.com/user-attachments/files/23456453/repro.zip) with all files necessary to reproduce, for your convenience.)
dep/src/lib.rs:
```rust
use proc_macro::TokenStream;
use std::sync::atomic::{AtomicI32, Ordering::SeqCst};
static COUNTER: AtomicI32 = AtomicI32::new(0);
#[proc_macro]
pub fn make_type(_: TokenStream) -> TokenStream {
let index = COUNTER.fetch_add(1, SeqCst);
match index {
0..=2 => "i32",
3 => "&'static i32",
_ => panic!(),
}
.parse()
.unwrap()
}
```
src/main.rs:
```rust
use dep::make_type;
use zerocopy::FromBytes;
#[derive(FromBytes)]
struct Thing {
field: make_type!(),
}
fn main() {
let thing: Thing = Thing::read_from_bytes(&1usize.to_ne_bytes()).unwrap();
let field: &'static i32 = thing.field;
println!("{field}");
}
```
Macro expansion
```rust
#![feature(prelude_import)]
#[macro_use]
extern crate std;
#[prelude_import]
use std::prelude::rust_2024::*;
use dep::make_type;
use zerocopy::FromBytes;
struct Thing {
field: &'static i32,
}
#[allow(deprecated)]
#[automatically_derived]
unsafe impl ::zerocopy::TryFromBytes for Thing<> where
i32: ::zerocopy::TryFromBytes {
fn only_derive_is_allowed_to_implement_this_trait() {}
fn is_bit_valid<___ZerocopyAliasing>(_candidate:
::zerocopy::Maybe)
-> ::zerocopy::util::macro_util::core_reexport::primitive::bool where
___ZerocopyAliasing: ::zerocopy::pointer::invariant::Reference {
if false {
fn assert_is_from_bytes() where T: ::zerocopy::FromBytes,
T: ?::zerocopy::util::macro_util::core_reexport::marker::Sized {}
assert_is_from_bytes::();
}
true
}
}
#[allow(deprecated)]
#[automatically_derived]
unsafe impl ::zerocopy::FromZeros for Thing<> where i32: ::zerocopy::FromZeros
{
fn only_derive_is_allowed_to_implement_this_trait() {}
}
#[allow(deprecated)]
#[automatically_derived]
unsafe impl ::zerocopy::FromBytes for Thing<> where i32: ::zerocopy::FromBytes
{
fn only_derive_is_allowed_to_implement_this_trait() {}
}
fn main() {
let thing: Thing = Thing::read_from_bytes(&1usize.to_ne_bytes()).unwrap();
let field: &'static i32 = thing.field;
{ ::std::io::_print(format_args!("{0}\n", field)); };
}
```
The `make_type!()` macro expands to `i32` three times, and `&'static i32` once. Turns out that the order of macro expansion ends up causing the `field` field to be defined as a `&'static i32`, but zerocopy instead checks whether `i32` implements `FromBytes`. The zerocopy derive macro then implements the `FromBytes` on the `Thing` type, which can then be exploited to cause UB.
The [same issue](https://github.com/Lokathor/bytemuck/issues/335) exists in bytemuck.
For the issue about this as a systematic problem, see https://github.com/rust-lang/rust/issues/148793.
See also https://github.com/rust-lang/rust/issues/147103, for a similar issue in std, although that case didn't cause unsoundness.
Contributor guide
Assessment
This issue has not been assessed yet.