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

Soundness hole: Use-after-free through mismatching lifetimes of stream item and yielded type #107

Open
steffahn opened this issue Apr 22, 2024 · 0 comments

Comments

@steffahn
Copy link

steffahn commented Apr 22, 2024

Soundness hole: Use-after-free through mismatching lifetimes of stream item and yielded type

This is almost trivially simple to run into. Just yield something that's shorter-lived than what the place using the stream requires, and … it just compiles.

(The only “trick” to avoiding compiler errors with this is that one needs to have the stream itself must not be kept alive for too long; i.e. significantly shorter than the items one gets from it.)

use async_stream::stream;
use futures::StreamExt;

use std::pin::pin;

#[tokio::main]
async fn main() {
    let mut outer = vec![];
    {
        let v = vec![0; 10];
        let v_ref = &v;
        let mut s = pin!(stream! {
            for x in v_ref {
                yield x
            }
        });
        while let Some(x) = s.next().await {
            outer.push(x);
        }
    };
    // use-after-free
    println!("{outer:?}"); // […garbage allocator internals…, 0, 0, 0]
}

(run on rustexplorer.com)

The relevant problem here is likely the covariance of the Sender type; I'm not 100% sure I understand what part of the expanded code currently allows for the corresponding unsound coercion to happen, nor do I know whether this was always accepted by the compiler, but even before that, the wrong covariance would've been not future-proof.


In my experience, this Sender/Receiver helper type pair that's supposed to ensure the yielded type is matching the item type might as well be unified into a single type, anyway. Especially after #106 is fixed, there's not really any point in involving a mutable borrow into this, it could simply be a simple Copy marker type that serves both roles of the current __yield_tx and __yield_rx (and .send method gets a by-value fn …(self, …) argument). The PhantomData inside would need to be invariant then, e.g. via PhantomData<fn(T) -> T>.

A minimal change on the other hand would be to just make the Sender<T> invariant or contravariant in <T>. With such a fix, we can gain the desired compilation error:

error[E0597]: `v` does not live long enough
  --> src/main.rs:11:21
   |
10 |         let v = vec![0; 10];
   |             - binding `v` declared here
11 |         let v_ref = &v;
   |                     ^^ borrowed value does not live long enough
...
20 |     };
   |     - `v` dropped here while still borrowed
21 |     println!("{outer:?}");
   |               --------- borrow later used here
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

No branches or pull requests

1 participant