`yoke` soundness issue with non-`'static` and contravariant cart, using `Yoke::attach_to_cart` + `Yoke::get`.

[steffahn]

Here’s a repro, in small steps with useful type annotations

use yoke::Yoke;

type F<'a> = fn(&'a ());

fn main() {
    let n = String::from("Hello World!");
    let x: Yoke<&'static str, Box<F<'_>>> = Yoke::attach_to_cart(Box::new((|_| {}) as _), |_| &n[..]);
    let y: Yoke<&'static str, Box<F<'static>>> = x;
    let s: &'static Yoke<&'static str, Box<F<'static>>> = Box::leak(Box::new(y));
    let r: &'static str = s.get();

    println!("pre-drop: {r}");
    drop(n);
    println!("post-drop: {r}");
}

Run Online in Rust Explorer

pre-drop: Hello World!
post-drop: �[YU��Es

[robertbastian]

@Manishearth

[Manishearth]

Oh, that's pretty insidious.

Some easy fixes would be to either restrict carts to 'static (then we lose the abiltiy to have borrowed carts, which I don't want to lose), or to restrict carts to some unsafe Cart trait (that woudl also fix #2095)

However I think we might be able to handle this by PhantomData'ing the cart so that it's forced to be covariant (which when mixed with contravariance, leads to invariance, so we're golden)

[Manishearth]

I have a fix that forces things to be invariant, which basically stops the step from x to y.

However, I'm somewhat uneasy because I'm not convinced that the attach_to_cart call should compile in the first place: This may be a compiler bug.

This is a very subtle interaction between variance, HRTBs, and well-formedness, and WFness isn't documented well enough for me to be able to give a clear answer, but I'm investigating by poking around and I'll see what I find.

[steffahn]

Forcing the cart to be invariant should indeed fix the problem; forcing "covariance" in the right sense - while perhaps more desirable (since it's less restrictive) - is not really possible with the yoke API, AFAICT. The cart's type parameter itself already is covariant in Yoke, the problem is determining whether the type of the cart itself contains contravariant lifetimes.

[steffahn]

For context: this issue in a different crate had a comparable underlying problem

Voultapher/self_cell#18

but their API involves a macro call that can know about all the lifetime parameters of the (equivalent of the) cart type, so forcing covariance (or else making the cart invariant) is as easy as adding another field covariantly using all the lifetimes of the cart.

[steffahn]

Another way to fix this issue would be to introduce a (covariant) lifetime argument for Yoke as in Yoke<'a, Y, C> and add a C: 'a bound to Yoke.

[Manishearth]

forcing "covariance" in the right sense - while perhaps more desirable - is not really possible (since it's less restrictive) with the yoke API

Yep, Rust does let you force all lifetimes in a parameter to be invariant, but it does not let you force them to be "at most covariant", since Rust's variance tools are the ability to:

• create covariant lifetimes (using references)
• force invariance on lifetimes (using cells/mut)
• pass through the variance of lifetimes (using simple wrappers)
• invert the variance of lifetimes (using fn parameters)

So the only tools that let you change variance are the ones that invert it (we don't want that), and the ones that force invariance.

---

But anyway, I'm now convinced this is a compiler/language bug (likely related to rust-lang/rust#25860), and am in the process of filing one. Might still land a fix here since it's a complex bug that may never get fixed.

[steffahn]

I'm not seeing any compiler bugs here, but feel free to elaborate, and I can add my interpretation of the situation when I have more time at hand, later today.

[Manishearth]

@steffahn Bug filed at rust-lang/rust#106431

The bug is that your attach_to_cart call should never have compiled: attach_to_cart's signature requires the closure to work for all 'de, and the closure clearly only works for 'de that is shorter than or equal to the lifetime of s.

My mitigation tactic (force all lifetimes in the cart to be treated as invariant) prevents such a contravariant carted Yoke from being used maliciously after the Yoke is created; however the Yoke should not have been possible to create in the first place.

[Manishearth]

Another potential fix would be to make C::Target: 'static in the attach_to_cart bounds. This means that:

• &'a [u8] still works as a cart in attach_to_cart() (but Box<&'a [u8]> does not)
• complex carts with borrows are still allowed (unsafely, via replace_cart and friends)
• carts retain variance

This gives maximum flexibility but disallows a couple things (that I personally don't care about) from going through the attach_to_cart signature (but they are still possible). OTOH the current proposed solution retains maximum flexibility of attach_to_cart, but then we get an insurmountable variance restriction.

Either way, let me make my PR with the current fix, and we can debate whether we should force invariance on carts or force C::Target: 'static in the attach signature.