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

object-safe traits can have associated types with unchecked bounds #27675

Closed
arielb1 opened this issue Aug 11, 2015 · 12 comments
Closed

object-safe traits can have associated types with unchecked bounds #27675

arielb1 opened this issue Aug 11, 2015 · 12 comments
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Aug 11, 2015

STR

trait Foo { type Assoc: PartialEq<Box<Self>>; }
impl Foo for u32 { type Assoc = Box<u32>; }

fn foo<T: Foo+?Sized>(u: Box<T>, v: T::Assoc) -> bool {
    &v == &u
}

fn main() {
    let bar: Box<Foo<Assoc=Box<u32>>> = Box::new(4);
    foo(bar, Box::new(5));
}

Result

<anon>:5:5: 5:13 error: internal compiler error: Encountered errors `[FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<Box<u32> as core::cmp::PartialEq<Box<Foo<Assoc=Box<u32>> + 'static>>>)),depth=1),Unimplemented)]` fulfilling during trans

I guess this should be banned.

cc @nikomatsakis

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 11, 2015

Related:

use std::fmt;

trait Foo {
    type Assoc: 'static;
}

fn foo<T: Foo+?Sized>(t: T::Assoc) -> Box<fmt::Display+'static>
        where T::Assoc: fmt::Display {
    Box::new(t)
}

fn wat() -> Box<fmt::Display+'static> {
    let x = 42;
    foo::<Foo<Assoc=&u32>>(&x)
}

fn main() {
    println!("{}", wat());
}

@nikomatsakis
Copy link
Contributor

Interesting. I agree that it should be illegal and we should amend WF relations for object types to check that the bindings cover the requirements of the trait -- the RFC was incomplete on this topic.

@pnkfelix
Copy link
Member

The example from the first comment (#27675 (comment)) is actually scarier than the original bug description; at least, I usually can think "oh, an ICE, well that might just be something that won't actually ever compile"; but the first comment is showing code that is indeed a case of "wat"!

@arielb1 arielb1 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Jan 21, 2016
@arielb1 arielb1 changed the title object-safe traits can have associated types with Self-containing bounds object-safe traits can have associated types with unchecked bounds Jan 21, 2016
@pnkfelix
Copy link
Member

pnkfelix commented Jul 7, 2016

triage: P-medium

@rust-highfive rust-highfive added the P-medium Medium priority label Jul 7, 2016
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 15, 2019
@Elinvynia Elinvynia added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@HeroicKatora
Copy link
Contributor

Since this has not been demonstrated: We can indeed build a safe, arbitrary transmutation primitive from this, though this will ICE in practice except if used only for the purpose of the second comment, casting away lifetimes.

trait Id<T>: Sized {
    fn id(self) -> T;
}
impl<T> Id<T> for T {
    fn id(self) -> T { self }
}

trait Setup<T> {
    type From: Id<T>;
}

fn transmute<T, U: Setup<T> + ?Sized>(from: U::From) -> T {
    Id::id(from)
}

// compiles fine
pub fn safe_transmute<T, U>(t: T) -> U {
    transmute::<U, dyn Setup<U, From=T>>(t)
}
fn main() {
    let static_word = {
        let st = String::from("Hello!");
        safe_transmute::<_, &'static mut str>(&*st)
    };
    
    println!("{:?}", static_word);
}
// ICE
fn main() {
    safe_transmute::<usize, &'static str>(0usize);
}

@HeroicKatora
Copy link
Contributor

HeroicKatora commented Sep 22, 2020

Uuups, we can make any type Copy. That's quite severe, wouldn't you say?

trait Setup {
    type From: Copy;
}

fn copy<U: Setup + ?Sized>(from: &U::From) -> U::From {
    *from
}

pub fn copy_any<T>(t: &T) -> T {
    copy::<dyn Setup<From=T>>(t)
}

fn main() {
    let st = String::from("Hello");
    copy_any(&st);
}

@lcnr lcnr removed the P-medium Medium priority label Sep 22, 2020
@jonas-schievink jonas-schievink added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 22, 2020
@HeroicKatora
Copy link
Contributor

HeroicKatora commented Sep 22, 2020

Feature unsize gives the ability to do some other interesting coercions that enable arbitrary sized reference transmute.

#![feature(unsize)]

trait Setup<T> {
    type From: std::marker::Unsize<[T]>;
}

fn unsize<T, U: Setup<T> + ?Sized>(from: &U::From) -> &[T] {
    from
}

fn unsize_to_any<T, U>(t: &T) -> &U {
    &unsize::<U, dyn Setup<U, From=[T]>>(core::slice::from_ref(t))[0]
}

fn main() {
    let trust_me_im_64: u8 = 0;
    println!("{}", unsize_to_any::<u8, u64>(&trust_me_im_64))
}

@camelid camelid added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Sep 22, 2020
@mstallmo mstallmo added P-high High priority I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ and removed I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 22, 2020
@camelid
Copy link
Member

camelid commented Sep 22, 2020

Decided on P-high as discussed in the prioritization working group procedure.

@HeroicKatora
Copy link
Contributor

HeroicKatora commented Sep 22, 2020

For completeness sake. This may be used to send any reference across threads:

trait Id<T> {
    fn id(&self) -> &T;
}

impl<T> Id<T> for T {
    fn id(&self) -> &T { self }
}

trait SyncSetup<T> {
    type From: Id<T> + Sync + 'static;
}

fn cast<T, U>(from: &T) -> &(dyn Id<U> + Sync) {
    fn do_it<T, U: ?Sized + SyncSetup<T>>(val: &U::From) -> &(dyn Id<T> + Sync) {
        val
    }
    
    do_it::<U, dyn SyncSetup<U, From=T>>(from)
}

fn syncify<T>(from: &T) -> &'static (dyn Id<T> + Sync) {
    // Very carefully avoid ICE. This only adds marker trait to an unsized trait.
    let x: &(dyn Id<T> + Sync) = cast(from);
    // Indirect over another reference and make `x` appear `&'static`.
    // Lifetime change is invisible to trait materialization checks.
    let y: &(dyn Id<&'static (dyn Id<T> + Sync)>) = cast(&x);
    // .. and grab that one.
    let x: &'static (dyn Id<T> + Sync) = *(y.id());
    x
}

use std::cell::Cell;
fn main() {
    let st = Cell::new(0u32);
    let x = syncify(&st);
    
    std::thread::spawn(move || {
        x.id().set(1);
    });
    
    while st.get() == 0 {}
    println!("Spooky");
}

@Aaron1011
Copy link
Member

I suspect that this will be fixed by #73905

@lqd
Copy link
Member

lqd commented Sep 24, 2020

The 4 weaponized examples above will indeed be rejected with #73905

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Oct 8, 2020
…-object-safe, r=Aaron1011

Add compile fail test for issue 27675

A recently merged PR (rust-lang#73905) strengthened the checks on bounds of associated items. This rejects the attack path of rust-lang#27675 which consisted of constructing a `dyn Trait<Item=T>` where `T` would not fulfill the bounds required on `Item` of the `Trait` behind the dyn object.

This regression test, extracted from [the weaponized instance](rust-lang#27675 (comment)), checks that this is rejected.
@Aaron1011
Copy link
Member

A regression test was added in #77663.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this issue Nov 24, 2020
Clean up some of the pkgsrc Makefile, there's still lots in here that
should just be deleted though.  Switch SunOS to the illumos bootstrap
by default.

Version 1.48.0 (2020-11-19)
==========================

Language
--------

- [The `unsafe` keyword is now syntactically permitted on modules.][75857] This
  is still rejected *semantically*, but can now be parsed by procedural macros.

Compiler
--------
- [Stabilised the `-C link-self-contained=<yes|no>` compiler flag.][76158] This tells
  `rustc` whether to link its own C runtime and libraries or to rely on a external
  linker to find them. (Supported only on `windows-gnu`, `linux-musl`, and `wasi` platforms.)
- [You can now use `-C target-feature=+crt-static` on `linux-gnu` targets.][77386]
  Note: If you're using cargo you must explicitly pass the `--target` flag.
- [Added tier 2\* support for `aarch64-unknown-linux-musl`.][76420]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`io::Write` is now implemented for `&ChildStdin` `&Sink`, `&Stdout`,
  and `&Stderr`.][76275]
- [All arrays of any length now implement `TryFrom<Vec<T>>`.][76310]
- [The `matches!` macro now supports having a trailing comma.][74880]
- [`Vec<A>` now implements `PartialEq<[B]>` where `A: PartialEq<B>`.][74194]
- [The `RefCell::{replace, replace_with, clone}` methods now all use `#[track_caller]`.][77055]

Stabilized APIs
---------------
- [`slice::as_ptr_range`]
- [`slice::as_mut_ptr_range`]
- [`VecDeque::make_contiguous`]
- [`future::pending`]
- [`future::ready`]

The following previously stable methods are now `const fn`'s:

- [`Option::is_some`]
- [`Option::is_none`]
- [`Option::as_ref`]
- [`Result::is_ok`]
- [`Result::is_err`]
- [`Result::as_ref`]
- [`Ordering::reverse`]
- [`Ordering::then`]

Cargo
-----

Rustdoc
-------
- [You can now link to items in `rustdoc` using the intra-doc link
  syntax.][74430] E.g. ``/// Uses [`std::future`]`` will automatically generate
  a link to `std::future`'s documentation. See ["Linking to items by
  name"][intradoc-links] for more information.
- [You can now specify `#[doc(alias = "<alias>")]` on items to add search aliases
  when searching through `rustdoc`'s UI.][75740]

Compatibility Notes
-------------------
- [Promotion of references to `'static` lifetime inside `const fn` now follows the
  same rules as inside a `fn` body.][75502] In particular, `&foo()` will not be
  promoted to `'static` lifetime any more inside `const fn`s.
- [Associated type bindings on trait objects are now verified to meet the bounds
  declared on the trait when checking that they implement the trait.][27675]
- [When trait bounds on associated types or opaque types are ambiguous, the
  compiler no longer makes an arbitrary choice on which bound to use.][54121]
- [Fixed recursive nonterminals not being expanded in macros during
  pretty-print/reparse check.][77153] This may cause errors if your macro wasn't
  correctly handling recursive nonterminal tokens.
- [`&mut` references to non zero-sized types are no longer promoted.][75585]
- [`rustc` will now warn if you use attributes like `#[link_name]` or `#[cold]`
  in places where they have no effect.][73461]
- [Updated `_mm256_extract_epi8` and `_mm256_extract_epi16` signatures in
  `arch::{x86, x86_64}` to return `i32` to match the vendor signatures.][73166]
- [`mem::uninitialized` will now panic if any inner types inside a struct or enum
  disallow zero-initialization.][71274]
- [`#[target_feature]` will now error if used in a place where it has no effect.][78143]
- [Foreign exceptions are now caught by `catch_unwind` and will cause an abort.][70212]
  Note: This behaviour is not guaranteed and is still considered undefined behaviour,
  see the [`catch_unwind`] documentation for further information.

Internal Only
-------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc and
related tools.
- [Building `rustc` from source now uses `ninja` by default over `make`.][74922]
  You can continue building with `make` by setting `ninja=false` in
  your `config.toml`.
- [cg_llvm: `fewer_names` in `uncached_llvm_type`][76030]
- [Made `ensure_sufficient_stack()` non-generic][76680]

[78143]: rust-lang/rust#78143
[76680]: rust-lang/rust#76680
[76030]: rust-lang/rust#76030
[70212]: rust-lang/rust#70212
[27675]: rust-lang/rust#27675
[54121]: rust-lang/rust#54121
[71274]: rust-lang/rust#71274
[77386]: rust-lang/rust#77386
[77153]: rust-lang/rust#77153
[77055]: rust-lang/rust#77055
[76275]: rust-lang/rust#76275
[76310]: rust-lang/rust#76310
[76420]: rust-lang/rust#76420
[76158]: rust-lang/rust#76158
[75857]: rust-lang/rust#75857
[75585]: rust-lang/rust#75585
[75740]: rust-lang/rust#75740
[75502]: rust-lang/rust#75502
[74880]: rust-lang/rust#74880
[74922]: rust-lang/rust#74922
[74430]: rust-lang/rust#74430
[74194]: rust-lang/rust#74194
[73461]: rust-lang/rust#73461
[73166]: rust-lang/rust#73166
[intradoc-links]: https://doc.rust-lang.org/rustdoc/linking-to-items-by-name.html
[`catch_unwind`]: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html
[`Option::is_some`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.is_some
[`Option::is_none`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.is_none
[`Option::as_ref`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.as_ref
[`Result::is_ok`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.is_ok
[`Result::is_err`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.is_err
[`Result::as_ref`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.as_ref
[`Ordering::reverse`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.reverse
[`Ordering::then`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.then
[`slice::as_ptr_range`]: https://doc.rust-lang.org/std/primitive.slice.html#method.as_ptr_range
[`slice::as_mut_ptr_range`]: https://doc.rust-lang.org/std/primitive.slice.html#method.as_mut_ptr_range
[`VecDeque::make_contiguous`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.make_contiguous
[`future::pending`]: https://doc.rust-lang.org/std/future/fn.pending.html
[`future::ready`]: https://doc.rust-lang.org/std/future/fn.ready.html
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jan 1, 2021
Pkgsrc changes:
 * Compensate for files being moved around upstream.
 * Introduce optional, on-by-default semi-static building of cargo,
   using the internal curl and openssl sources.  This reduces the dynamic
   dependencies of cargo and therefore the rust package itself.
   Ref. options.mk.
 * The 1.47.0 bootstrap kits have been re-built with the above option
   turned on, so no longer depends on curl or openssl from pkgsrc and/or
   from earlier OS or pkgsrc versions.  This should hopefully fix
   installation of rust with non-default PREFIX, ref. PR#54453.


Upstream changes:

Version 1.48.0 (2020-11-19)
==========================

Language
--------
- [The `unsafe` keyword is now syntactically permitted on modules.][75857] This
  is still rejected *semantically*, but can now be parsed by procedural macros.

Compiler
--------
- [Stabilised the `-C link-self-contained=<yes|no>` compiler flag.][76158]
  This tells `rustc` whether to link its own C runtime and libraries
  or to rely on a external linker to find them. (Supported only on
  `windows-gnu`, `linux-musl`, and `wasi` platforms.)
- [You can now use `-C target-feature=+crt-static` on `linux-gnu` targets.]
  [77386]
  Note: If you're using cargo you must explicitly pass the `--target` flag.
- [Added tier 2\* support for `aarch64-unknown-linux-musl`.][76420]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`io::Write` is now implemented for `&ChildStdin` `&Sink`, `&Stdout`,
  and `&Stderr`.][76275]
- [All arrays of any length now implement `TryFrom<Vec<T>>`.][76310]
- [The `matches!` macro now supports having a trailing comma.][74880]
- [`Vec<A>` now implements `PartialEq<[B]>` where `A: PartialEq<B>`.][74194]
- [The `RefCell::{replace, replace_with, clone}` methods now all use
  `#[track_caller]`.][77055]

Stabilized APIs
---------------
- [`slice::as_ptr_range`]
- [`slice::as_mut_ptr_range`]
- [`VecDeque::make_contiguous`]
- [`future::pending`]
- [`future::ready`]

The following previously stable methods are now `const fn`'s:

- [`Option::is_some`]
- [`Option::is_none`]
- [`Option::as_ref`]
- [`Result::is_ok`]
- [`Result::is_err`]
- [`Result::as_ref`]
- [`Ordering::reverse`]
- [`Ordering::then`]

Cargo
-----

Rustdoc
-------
- [You can now link to items in `rustdoc` using the intra-doc link
  syntax.][74430] E.g. ``/// Uses [`std::future`]`` will automatically generate
  a link to `std::future`'s documentation. See ["Linking to items by
  name"][intradoc-links] for more information.
- [You can now specify `#[doc(alias = "<alias>")]` on items to add
  search aliases when searching through `rustdoc`'s UI.][75740]

Compatibility Notes
-------------------
- [Promotion of references to `'static` lifetime inside `const fn`
  now follows the same rules as inside a `fn` body.][75502] In
  particular, `&foo()` will not be promoted to `'static` lifetime
  any more inside `const fn`s.
- [Associated type bindings on trait objects are now verified to meet the bounds
  declared on the trait when checking that they implement the trait.][27675]
- [When trait bounds on associated types or opaque types are ambiguous, the
  compiler no longer makes an arbitrary choice on which bound to use.][54121]
- [Fixed recursive nonterminals not being expanded in macros during
  pretty-print/reparse check.][77153] This may cause errors if your macro wasn't
  correctly handling recursive nonterminal tokens.
- [`&mut` references to non zero-sized types are no longer promoted.][75585]
- [`rustc` will now warn if you use attributes like `#[link_name]` or `#[cold]`
  in places where they have no effect.][73461]
- [Updated `_mm256_extract_epi8` and `_mm256_extract_epi16` signatures in
  `arch::{x86, x86_64}` to return `i32` to match the vendor signatures.][73166]
- [`mem::uninitialized` will now panic if any inner types inside
  a struct or enum disallow zero-initialization.][71274]
- [`#[target_feature]` will now error if used in a place where it
  has no effect.][78143]
- [Foreign exceptions are now caught by `catch_unwind` and will
  cause an abort.][70212] Note: This behaviour is not guaranteed
  and is still considered undefined behaviour, see the [`catch_unwind`]
  documentation for further information.

Internal Only
-------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc and
related tools.

- [Building `rustc` from source now uses `ninja` by default over `make`.][74922]
  You can continue building with `make` by setting `ninja=false` in
  your `config.toml`.
- [cg_llvm: `fewer_names` in `uncached_llvm_type`][76030]
- [Made `ensure_sufficient_stack()` non-generic][76680]

[78143]: rust-lang/rust#78143
[76680]: rust-lang/rust#76680
[76030]: rust-lang/rust#76030
[70212]: rust-lang/rust#70212
[27675]: rust-lang/rust#27675
[54121]: rust-lang/rust#54121
[71274]: rust-lang/rust#71274
[77386]: rust-lang/rust#77386
[77153]: rust-lang/rust#77153
[77055]: rust-lang/rust#77055
[76275]: rust-lang/rust#76275
[76310]: rust-lang/rust#76310
[76420]: rust-lang/rust#76420
[76158]: rust-lang/rust#76158
[75857]: rust-lang/rust#75857
[75585]: rust-lang/rust#75585
[75740]: rust-lang/rust#75740
[75502]: rust-lang/rust#75502
[74880]: rust-lang/rust#74880
[74922]: rust-lang/rust#74922
[74430]: rust-lang/rust#74430
[74194]: rust-lang/rust#74194
[73461]: rust-lang/rust#73461
[73166]: rust-lang/rust#73166
[intradoc-links]: https://doc.rust-lang.org/rustdoc/linking-to-items-by-name.html
[`catch_unwind`]: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html
[`Option::is_some`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.is_some
[`Option::is_none`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.is_none
[`Option::as_ref`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.as_ref
[`Result::is_ok`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.is_ok
[`Result::is_err`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.is_err
[`Result::as_ref`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.as_ref
[`Ordering::reverse`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.reverse
[`Ordering::then`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.then
[`slice::as_ptr_range`]: https://doc.rust-lang.org/std/primitive.slice.html#method.as_ptr_range
[`slice::as_mut_ptr_range`]: https://doc.rust-lang.org/std/primitive.slice.html#method.as_mut_ptr_range
[`VecDeque::make_contiguous`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.make_contiguous
[`future::pending`]: https://doc.rust-lang.org/std/future/fn.pending.html
[`future::ready`]: https://doc.rust-lang.org/std/future/fn.ready.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.