Repository navigation
Description
edit: while this issue has been mostly fixed by #115538, the underlying issue still exists for higher ranked function pointers. It feels highly likely that it can still be exploited, even if the exploit will now be even more involved.
Make sure to also read through the next few comments of mine, where I present some other, significantly different examples of related problems (that’s where HRTBs become relevant, and closures, fn-pointers and trait objects play a role). I also explain the T-lang tag there with a question on changing the meaning of HRTBs that include associated types. Possibly it’s even a sensible thing to split this issue up into two issues.
So if I write a function such as
fn foo<'b, 'a>() -> PhantomData<&'b &'a ()> {
PhantomData
}then foo’s return type is only valid if 'a: 'b. When we call the function this is enforced a the call site, e.g.
fn caller<'b, 'a>() {
foo::<'b, 'a>();
}doesn’t work, with
error[E0491]: in type `&'b &'a ()`, reference has a longer lifetime than the data it references
--> src/lib.rs:8:5
|
8 | foo::<'b, 'a>();
| ^^^^^^^^^^^^^^^
|
note: the pointer is valid for the lifetime `'b` as defined on the function body at 7:11
--> src/lib.rs:7:11
|
7 | fn caller<'b, 'a>() {
| ^^
note: but the referenced data is only valid for the lifetime `'a` as defined on the function body at 7:15
--> src/lib.rs:7:15
|
7 | fn caller<'b, 'a>() {
| ^^
error: aborting due to previous errorHowever, if we just instantiate the lifetime parameters, there’s apparently no check being performed at all, i.e.
fn caller<'b, 'a>() {
foo::<'b, 'a>;
}compiles just fine. (playground).
This seems questionable, the type of foo::<'b, 'a> implements FnOnce with Output = PhantomData<&'b &'a ()>, and it is in fact possible to use this output type to circumvent the borrow checker:
use std::marker::PhantomData;
fn foo<'b, 'a>() -> PhantomData<&'b &'a ()> {
PhantomData
}
fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
let f = foo::<'b, 'a>;
f.baz(x)
}
trait Foo<'a, 'b, T: ?Sized> {
fn baz(self, s: &'a T) -> &'b T;
}
impl<'a, 'b, R, F, T: ?Sized> Foo<'a, 'b, T> for F
where
F: Fn() -> R,
R: ProofForConversion<'a, 'b, T>,
{
fn baz(self, s: &'a T) -> &'b T {
self().convert(s)
}
}
trait ProofForConversion<'a, 'b, T: ?Sized> {
fn convert(self, s: &'a T) -> &'b T;
}
impl<'a, 'b, T: ?Sized> ProofForConversion<'a, 'b, T> for PhantomData<&'b &'a ()> {
fn convert(self, s: &'a T) -> &'b T {
s
}
}
fn main() {
let d;
{
let x = "Hello World".to_string();
d = extend_lifetime(&x);
}
println!("{}", d);
}�ZV&�V�
@rustbot modify labels: A-lifetimes, A-typesystem, T-compiler
and someone please add “I-unsound 💥”.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsnew solver everywhere
Activity
SkiFire13 commentedon Apr 25, 2021
Possible duplicate of #25860?
steffahn commentedon Apr 25, 2021
I wouldn't think it's a duplicate but I guess it's related, at least in the sense that there are changes to Rust that could solve both issues at once. This bug results in unsoundness even if the invalidly typed function is never called. (It is called in my example, but I think it can be refactored so that it doesn't get called.
I will try to create some example code later today.Edit: Here’s a playground.) The point in this case is that the type offoo::<'b, 'a>implementsFntraits with anOutputtype whose validity hasn't been checked.I guess it's similar in spirit to #82633 in this sense. Possible resolutions are either to forbid instantiating
foo::<'b, 'a>with these lifetime parameters entirely or to at least make sure that the (anonymous function-)type of that instantiation doesn't implement anyFntraits. I guess I'll add @rustbot modify labels to +A-traits.steffahn commentedon Apr 25, 2021
Edit: This example does, unlike the issue I described above, involve HRTBs. You might qualify everything in this and the following 3 comments of mine as a seperate / new issue. I don’t like creating too many similar “I-unsound 💥” issues though, and the main problem is similar in that there’s a type that implements a trait with an illegal associated type. (Illegal in the sense that implied bounds are violated, in particular the type is
&'b &'a ()orPhantomData<&'b &'a ()>in a setting where'a: 'bmight not actually hold true.) I have since renamed this topic so that it fits all the examples.Here’s another problem involving illegal
Fnimplementations. Makes me think that it might be a good idea after all to have some function-types or function-pointer-types that don’t implement certainFn-traits.This might also be relevant to the discussion of #82633, where the current proposed resolution is to disallowEdit: On second thought, the situation described below is significantly different from #82633, so Iʼm not sure of the relevance of this issue for the discussion in #82633 anymore.fn-pointers with!Sizedreturn types, whereas an alternative approach could be to just have them not implementFn-traits.Onto the problem. Take trait bounds such as
for<'b, 'a> Fn(&'b(), &'a()) -> PhantomData<&'b &'a ()>. The corresponding function-pointer typefor<'b, 'a> fn(&'b(), &'a()) -> PhantomData<&'b &'a ()>implements this trait bound.However, if you try to re-create an
Fn-trait-style trait and try to implement it accordingly, this fails, and with good reason:In other words, the
Fn-implementation of these functions / function-pointers / closures is unsound. I’ll try to create a direct exploitation of this unsoundness later today.steffahn commentedon Apr 25, 2021
(playground)
steffahn commentedon Apr 25, 2021
By the way, next to function types and function pointer types and closure types, you also have
dyn for<'b, 'a> Fn(&'b (), &'a ()) -> PhantomData<&'b &'a ()>as a type that implementsfor<'b, 'a> Fn(&'b (), &'a ()) -> PhantomData<&'b &'a ()>.Here’s an exploitation of that trait object type:
To reiterate, having any type implement
for<'b, 'a> Fn(&'b (), &'a ()) -> PhantomData<&'b &'a ()>is currently unsound.We could talk about making a trait bound
for<'b, 'a> Fn(&'b (), &'a ()) -> PhantomData<&'b &'a ()>mean something different, i.e. it could be made to only quantify over those'band'asuch thatPhantomData<&'b &'a ()>, too, is a valid type, i.e. such that'a: 'bholds.However, after such a change, we would need to make sure that
cannot imply
anymore. And accordingly, trait inference would need to be changed so that e.g. the
F: for<'b, 'a> Fn<(&'b (), &'a ()), Output = PhantomData<&'b &'a ()>>doesn’t implyF: HelperTrait<'b1, 'a1>anymore, unless'a1: 'b1. (HelperTraithaving a blanket implementation like e.g. in one of my code examples above.)I’d guess that answering the question of how to best solve this issue (especially the question of changing the meaning of HRTBs with associated types) might touch on T-lang responsibility. I’m not entirely sure if that’s the correct procedure while there’s an outstanding I-prioritize for T-compiler, but I’ll just add the label – @rustbot modify labels: +T-lang, +A-dst. The additional A-dst is for the trait object example.
[-]It’s possible to have the validity of a function return type never checked.[/-][+]Functions / closures / trait objects can implement traits such that validity of associated types is never checked.[/+]29 remaining items
Auto merge of rust-lang#115538 - lcnr:fn-def-wf, r=compiler-errors
asquared31415 commentedon Apr 4, 2024
The PR that closed this says "fixes #84533 (mostly)", should this be closed, or should it remain open due to the parts that were seemingly not fixed?
lcnr commentedon Apr 5, 2024
reopened it with a note and with a reduce priority 👍
Squashed commit of the following:
BoxyUwU commentedon Jan 1, 2025
I believe this is the same unsoundness already demonstrated in this thread but I find the way its written to be easier to follow:
noakodaren commentedon Mar 7, 2025
It would be nice if code like the below using implied bounds (https://sabrinajewson.org/blog/the-better-alternative-to-lifetime-gats) combined with trait objects would keep working after this issue get fixed: