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

late-bound lifetimes on type-alias-impl-trait can be unsound #97104

Closed
aliemjay opened this issue May 17, 2022 · 4 comments · Fixed by #98933
Closed

late-bound lifetimes on type-alias-impl-trait can be unsound #97104

aliemjay opened this issue May 17, 2022 · 4 comments · Fixed by #98933
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@aliemjay
Copy link
Member

aliemjay commented May 17, 2022

#![feature(type_alias_impl_trait)]

mod lifetime_params {
    type Ty<'a> = impl Sized + 'a;
    fn define(s: &str) -> Ty<'_> { s }

    type BadFnSig = fn(Ty<'_>) -> &str;
    type BadTraitRef = dyn Fn(Ty<'_>) -> &str;
}

mod type_params {
    type Ty<T> = impl Sized;
    fn define<T>(s: T) -> Ty<T> { s }

    type BadFnSig = fn(Ty<&str>) -> &str;
    type BadTraitRef = dyn Fn(Ty<&str>) -> &str;
}

Playground.

Prior to #96903, these types were rejected with this error:

error[E0581]: return type references an anonymous lifetime, which is not constrained by the fn input types
 --> /tmp/test9.rs:7:35
  |
7 |     type BadFnSig = fn(Ty<'_>) -> &str;
  |                                   ^^^^
  |
  = note: lifetimes appearing in an associated type are not considered constrained

Currently, they're accepted.

Based on the assumption that generic parameters on opaque type are not constrained (see Niko's comment #95474 (comment)), I believe the previous behavior is correct.

CC: @oli-obk

@rustbot label F-type_alias_impl_trait A-impl-trait T-compiler A-lifetimes

@aliemjay aliemjay added the C-bug Category: This is a bug. label May 17, 2022
@rustbot rustbot added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 17, 2022
@aliemjay
Copy link
Member Author

Out of curiosity, here's an exploit derived from #84366.

It type-checks but fails at monomorphization.

playground

#![feature(type_alias_impl_trait)]

trait Static: 'static {}
impl Static for () {}

type Gal<T> = impl Static;
fn _defining<T>() -> Gal<T> {}

trait Callable<Arg> { type Output; }

/// We can infer `<C as Callable<Arg>>::Output: 'static`,
/// because we know `C: 'static` and `Arg: 'static`,
fn box_str<C, Arg>(s: C::Output) -> Box<dyn AsRef<str> + 'static>
where
    Arg: Static,
    C: ?Sized + Callable<Arg> + 'static,
    C::Output: AsRef<str>,
{
    Box::new(s)
}

fn extend_lifetime(s: &str) -> Box<dyn AsRef<str> + 'static> {
    type MalformedTy = dyn for<'a> Callable<Gal<&'a ()>, Output = &'a str>;
    box_str::<MalformedTy, _>(s)
}

fn main() {
    //let extended = extend_lifetime(&String::from("hello"));
    //println!("{}", extended.as_ref().as_ref());
}

@aliemjay aliemjay changed the title late-bound lifetimes on type-alias-impl-trait are unsound late-bound lifetimes on type-alias-impl-trait can be unsound Jun 17, 2022
@aliemjay
Copy link
Member Author

The above exploit is check-pass and should be easier to do with #95474, where indirection through Static would not be necessary, so

@rustbot label I-unsound requires-nightly T-types

@rustbot rustbot added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Jun 17, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jun 23, 2022

Prior to #96903, these types were rejected with this error:

if the same code were written without lifetime params and used &'a () arguments instead, we'd see the same problem before #96903, right?

@aliemjay
Copy link
Member Author

aliemjay commented Jul 5, 2022

if the same code were written without lifetime params and used &'a () arguments instead, we'd see the same problem before #96903, right?

Sorry, I've clearly missed the notification 😅. I think you already know the answer given that you've just pushed a fix :).

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 8, 2022
…times, r=lcnr

Opaque types' generic params do not imply anything about their hidden type's lifetimes

fixes rust-lang#97104

cc `@aliemjay`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 8, 2022
…times, r=lcnr

Opaque types' generic params do not imply anything about their hidden type's lifetimes

fixes rust-lang#97104

cc ``@aliemjay``
@bors bors closed this as completed in d392838 Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Development

Successfully merging a pull request may close this issue.

3 participants