search

alloy-rs / alloy

Transports, Middleware, and Networks for the Alloy project
https://alloy.rs
Apache License 2.0
668 stars 243 forks source link

[Bug] Incorrect safety invariant #1722

Open SozinM opened 15 hours ago

SozinM commented 15 hours ago

Component

rpc

What version of Alloy are you on?

v0.7.0/master

Operating System

None

Describe the bug

Safety invariant in the unwrap function is incorrect https://github.com/alloy-rs/alloy/blob/67576f9f6e70026116443851a43e4c805902c3b3/crates/rpc-types-engine/src/payload.rs#L476 Structs https://github.com/alloy-rs/alloy/blob/67576f9f6e70026116443851a43e4c805902c3b3/crates/rpc-types-engine/src/payload.rs#L460 And https://github.com/alloy-rs/alloy/blob/67576f9f6e70026116443851a43e4c805902c3b3/crates/rpc-types-engine/src/payload.rs#L449 Have default repr (Rust) Rust repr does not guarantee that this structs would have the same memory layout (https://doc.rust-lang.org/nomicon/repr-rust.html). Also transmute highlights that it should not be used with Rust repr

When transmuting between different compound types, you have to make sure they are laid out the same way! If layouts differ, the wrong fields are going to get filled with the wrong data, which will make you unhappy and can also be Undefined Behavior (see above).

So how do you know if the layouts are the same? For repr(C) types and repr(transparent) types, layout is precisely defined. But for your run-of-the-mill repr(Rust), it is not.

https://doc.rust-lang.org/nomicon/transmutes.html

To fix the problem we could either change unwrap or add repr(C) to structs (but we will need to align them manually)

wrap_ref have similar issue too, but it's not easy to fix this as the operation itself is not safe (only with repr(C) ) so it's up to discussion

DaniPopes commented 5 hours ago

this is still present in wrap_ref