tokio-rs / async-stream

Asynchronous streams for Rust using async & await notation
Other
604 stars 32 forks source link

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

Open steffahn opened 4 months ago

steffahn commented 4 months ago

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