Associated type constraints on super traits allowing for unsound transmutation to trait objects

[Kyykek]

This code allows for unsound transmutation to trait objects of trait with impossible constraints.

trait TypeEq {
    type This: ?Sized;
}

impl<T: ?Sized> TypeEq for T {
    type This = T;
}

fn identity<T: ?Sized>(t: &<T as TypeEq>::This) -> &T {
    t  // T::This is deduced to be T
}

trait Cat {
    fn meow(&self) {
        println!("meow");
    }
}

struct SomeType;
impl Cat for SomeType {}

// `dyn Dog` isn't a valid type, but we are still allowed to create a trait object of this
trait Dog: TypeEq<This = dyn Cat> {
    fn bark(&self) {
        println!("bark");
    }
}

pub fn main() {
    let normal_coercion: &dyn Cat = &SomeType;

    // `T` is inferred to be `dyn Dog`, `T::This` is inferred as `dyn Cat`, which is different from how the compiler
	// inferred these types in the function definition.
    let oh_no_my_vtable: &dyn Dog = identity(normal_coercion);

    oh_no_my_vtable.bark(); // meow :3
}

I'm not sure if this is a special case of one of the numerous similar known bugs, so I thought I should share it just in case. Simpler cases where the types' memory layouts do not match cause the compiler to panic or llvm to segfault during optimization.

[asquared31415]

A trivial change allows a segfault in safe code:

trait Dog: TypeEq<This = dyn Cat> {
    fn bark(&self) {
        println!("bark");
    }

    fn extra(&self) {
        panic!("extra");
    }
}

// ... 

oh_no_my_vtable.extra(); // segfault

[Kyykek]

Note that that code does not segfault with rustc 1.71; it simply does nothing. 1.70 compiles it to a segfault.
Both 1.71 and 1.70 compile to a SIGILL when turning on optimizations (-O).

Compiler Explorer

[Kyykek]

Here is the compiler panic example (presumably caused by the transmutation of a thin pointer to a fat pointer)

trait TypeEq {
    type This: ?Sized;
}

impl<T: ?Sized> TypeEq for T {
    type This = Self;
}

fn identity<T: ?Sized>(t: &<T as TypeEq>::This) -> &T {
    t
}

struct SomeType;

// compiler crashes if called
fn unsound(x: &SomeType) -> &dyn TypeEq<This = SomeType> {
    identity(x)
}

fn main() {
    let thing = SomeType;
    unsound(&thing);  // crash!
}

Compiler Explorer

Here is the example that causes a libLLVM segfault when using -O

trait TypeEq {
    type This: ?Sized;
}

impl<T: ?Sized> TypeEq for T {
    type This = Self;
}

fn identity<T: ?Sized>(t: &<T as TypeEq>::This) -> &T {
    t
}

trait Dog: TypeEq<This = [u8]> {
    fn woof(&self) {
        println!("woof");
    }
}

fn main() {
    let arr = vec![1; 0];
    let arr: &[u8] = &arr; // fat pointer (ptr, len) with len being 0
    let dog: &dyn Dog = identity(arr);  // (data_ptr, vtable_ptr) with vtable_ptr having address 0 
	// (at least this is what i assume, idk how DSTs are actually 
	// represented when turned into trait objects) 
    dog.woof(); // llvm does not like this.
}

Compiler Explorer

Replacing vec![1; 0] with Vec::new() or just [1; 0] causes the output program to segfault instead. There seems to be some weird stuff going on when adding in the complexity of std::vec::from_elem() (the doc hidden function called by vec! in this case).
The segfault also happened when i first came across this while writing type level magic. The compiler just segfaulting on me without any sort of error message initially made me think that I had screwed up something with my environment.

[fmease]

@rustbot label -needs-triage T-compiler T-types I-unsound A-trait-objects A-associated-items A-coercions

[Kyykek]

This is very similar to #57893, in that an incorrect associated type bound is assumed correct.

[wwylele]

This can be exploited without involving super traits. The trick is to construct the impossible type dyn TypeEq<This = Anything>

trait TypeEq {
    type This: ?Sized;
    fn bark(&self) {
        println!("bark");
    }
}

impl<T: ?Sized> TypeEq for T {
    type This = T;
}

fn identity<T: ?Sized>(t: &<T as TypeEq>::This) -> &T {
    t  // T::This is deduced to be T
}

trait Cat {
    fn meow(&self) {
        println!("meow");
    }
}

struct SomeType;
impl Cat for SomeType {}

pub fn main() {
    let normal_coercion: &dyn Cat = &SomeType;

    let oh_no_my_vtable: &dyn TypeEq<This = dyn Cat> = identity(normal_coercion);

    oh_no_my_vtable.bark(); // meow :3
}

[fmease]

This is very similar to #57893, in that an incorrect associated type bound is assumed correct.

Hmm, indeed. I've quickly looked at open I-unsound issues earlier but didn't find anything related but this issue could actually be a duplicate of the one you linked.

Could somebody else take a look and confirm or refute that this is indeed a duplicate?
@rustbot label needs-triage -I-prioritize

[Kyykek]

Well, the fundamental issue at hand seems to be that if we say that dyn Tr automatically implements Tr, then inferring the type This from the impl body is simply unsound because it doesnt take into account all possible types that T and therefore This can take on.

impl<T: ?Sized> Tr for T does not implement Tr for dyn Tr because that would be a conflicting implementation, as can be seen when trying to impl Tr for dyn Tr. As such, dyn Tr<This=Impossible> does not contradict the implementation, because it doesn't apply to it.

Coercing SomeType into dyn TypeEq<This=SomeType> is unsound because the only reason that SomeType implements
TypeEq<This=SomeType> is that it fulfills T == SomeType for its implementation which is not the case for dyn TypeEq<This=SomeType>.

The underlying issue is that by inferring the associated type from the blanket implementation we assume that it applies to all types, when it simply does not because dyn Tr works differently.

If we want to infer types from impl blocks because "the implementation is always the same" it's not enough to just look at the impl.

fn identity<T: ?Sized>(t: &<T as TypeEq>::This) -> &T {
    t
}

I don't think this function should compile, since there are types that satisfy the trait bounds of T for which inferring that This == T is not correct.

In my eyes, #57893 and this have the same root cause, but I do not think that normalization as mentioned over there is enough to fully fix this issue.

[Kyykek]

Alternatively, one could require something like that to be valid (for a given equality bound), a trait object type like dyn TypeEq<This=T> needs to fulfill any implementation (i.e. any blanket implementation) whose conditions apply to the trait object type.
For example dyn TypeEq<This=Example> violates this condition because dyn TypeEq<This=Example> fulfills T: ?Sized but when substituting it for T, we get the false statement Example == dyn TypeEq<This=Example>. Therefore, dyn TypeEq<This=Example> is not a valid type.
If we don't require trait object types to agree with blanket implementations (at least those that would 'apply' to them), we pretty much cannot assume anything about the trait members of a generic parameter bound to that trait.

This also fixes the example from the earlier issue. I have no idea if this can be implemented though, maybe only in simple cases.

Edit: after writing this, i have found that someone has already made that suggestion here. I also think my issue should be closed as duplicate, but I'd like to hear another opinion.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

I'm going to nominate this issue for the next T-compiler meeting to get more eyeballs on this (and the related old issue mentioned). I'd like to understand better what's the current status of this unsoundness, how we can proceed and in which timeframe.

@rustbot label +I-compiler-nominated

[Kyykek]

I would also like to remind that more complex bounds might make restricting object safety without breaking sound code complicated.

trait Tr {
   type T: ?Sized;
}

trait ExtraBound {}

impl<T: ?Sized+ExtraBound> Tr for T {
   type T=T;
}

trait TrExtra: Tr+ExtraBound {}
impl<T: Tr+ExtraBound+?Sized> TrExtra for T {}

// this is sound since the blanket impl requires ExtraBound.
type Sound = dyn Tr<T=i32>;
// this is not
type Unsound = dyn TrExtra<T=i32>;

Though you could argue that the lack of object safety for TrExtra arises from its blanket impl, while Tr's blanket impl does not restrict it in that way.

This gets more interesting with dyn Tr+SomeAutoTrait

trait Tr {
   type T: ?Sized;
}
impl<T: ?Sized+Send> Tr for T {
   type T=T;
}
// no contradiction with blanket because no Send
type Sound = dyn Tr<T=i32>;
// but this is not sound even though both of its components are
type Unsound = dyn Tr<T=i32>+Send;

To handle this it would seem that more special treatment of auto traits is needed?

Also there's dyn Tr+'lifetime

trait Tr {
   type T: ?Sized;
}
impl<T: ?Sized+'static> Tr for T {
   type T=T;
}

type MightBeSound = dyn Tr<T=i32>;
type Unsound = dyn Tr<T=i32>+'static;

I do not even want to think about how something like

trait Tr<'a>: 'a {}

would play out here.

Or maybe I'm just pessimistic. I haven't thought that much about the exact details of which cases are actually problematic.

[apiraino]

Discussed in T-compiler meeting on Zulip. T-types will ignite a follow-up discussion on Zulip to evaluate the overlap with #57893 and also make the point about this perhaps being more feasible to accidentally trigger.

Removing the nomination for T-compiler for now.

@rustbot label -I-compiler-nominated

[Kyykek]

I saw some of the discussion on zulip, #57893 is still completely broken. I have tried most of the examples there and all I've tried still worked.
Also I can confirm that I have actually run into this on accident.
Furthermore, I'm (personally) 100% sure that this is just a special, more practical case of #57893. The core issue seems to be incoherent handling of associated types for blanket implementations whose conditions are fulfilled by the dyn Tr type itself. I.e. dyn Tr<T=T> can pass any type allowed for the associated type, but for type parameters with the blanket implementation's bounds, it is assumed the blanket implementation holds.

[nikomatsakis]

In today's t-types meeting, we decided to close this issue as a duplicate of #57893 -- we are going to place further comments in that issue.