hyper icon indicating copy to clipboard operation
hyper copied to clipboard

UpgradedSendStream transmute seems to be super unsound

Open Manishearth opened this issue 6 months ago • 12 comments

https://github.com/hyperium/hyper/blob/1d4ff3597b8e76818c8553dbfa4234cf4208c958/src/proto/h2/mod.rs#L389C8-L397

This isn't sound. Neutered<B> is:

#[repr(transparent)]
struct Neutered<B> {
    _inner: B,
    impossible: Impossible,
}

enum Impossible {}

Firstly, it's unclear what guarantees repr(transparent) confers here, impossible does have size_of == 0 but it's not exactly a zero-sized type (https://github.com/rust-lang/unsafe-code-guidelines/issues/485). The repr(transparent) RFC is not clear about this, and it's quite shaky ground to rely on, especially since empty ("impossible") types are one place Rust does sometimes perform layout optimizations. There's an assertion guard against that for Neutered but it's not clear how transitive this stuff is (https://github.com/rust-lang/unsafe-code-guidelines/issues/486)

Empty types can and do affect enum representations. SendBuf<B> has the following definition:

#[repr(usize)]
enum SendBuf<B> {
    Buf(B),
    Cursor(Cursor<Box<[u8]>>),
    None,
}

There's nothing preventing the compiler from realizing that Buf(Neutered<...>) is impossible and deciding to not include it as a discriminant choice. The asserts in this code are insufficient to check that.

This code seems to work fine under the current compiler but I wouldn't rely on that.

Manishearth avatar Dec 21 '23 16:12 Manishearth