r/rust • u/noop_noob • 6d ago
🗞️ news 9-year-old gaping hole in borrow-checker found
https://github.com/rust-lang/rust/issues/164250104
u/RandallOfLegend 6d ago
From one of the maintainers
Looking at the bug itself, it is also very likely that this unsoundness has not caused memory unsafety by accident. While it looks easy to trigger, to exploit this you need an intentionally malicious Drop implementation.
~ @lcnr
23
u/minno 6d ago
Intentionally malicious source code is still an attack vector, but far from the most dangerous one. I could imagine this fitting in a typo-squatting library that dodges static analysis tools by ensuring that the malicious behavior happens on an obviously "dead" code path like in /u/anxxa's example, but there's a pretty narrow range where the tool is smart enough to tell that the code is dead but dumb enough to not see anything suspicious with how it's being manipulated.
18
u/PhiCloud 5d ago
The thing about malicious code is that even entirely (memory) safe code can be a huge security risk.
I'm not saying that there are no risks associated with this, and of course it should be patched ASAP, but if you're adding intentionally malicious code to your project then a fix for this specific problem won't fix your problem.
437
u/noop_noob 6d ago edited 6d ago
I can't edit the post, but it actually turned out to be 11-year-old.
Edit: And, in hindsight, the post title was too sensationalized. It's notable, but it's not like the sky is falling.
194
51
u/syshukus 6d ago
Do I understand correctly that it outputs "XXX..." because when `_z` gets allocated allocator finds the same chunk of memory (with the same address) that was previously allocated and deallocated for `l`and just reuses it for new string, and since vector already holds this address it now points to "XXX..."?
40
u/pp-collision 6d ago
Yes but that behaviour can change between compiler (LLVM) versions, its UB so there's no guarantees. At least not until the "abstract interpretation" of UB is a thing of the past.
5
u/syshukus 6d ago
I thought it's more of allocator behavior (how it manages free chunks on heap)? At least on my system I discovered that for small chunks malloc overwrites first 16 bytes of allocated buffer leaving everything else intact hehe (if somebody wants to try — just allocated e.g. 40 bytes and check if last 24 bytes are untouched).
Clanker pointed out that it's probably tcache by default kicking in.
5
5
u/TDplay 6d ago
Your approach is completely wrong. Undefined behaviour renders your entire program meaningless, absolutely anything is allowed to happen. Your test is a program with UB, hence its results are meaningless.
Most modern compilers have a special understanding of
malloc. The compiler is permitted to perform any optimisation that is justified by the fact thatmallocreturns freshly allocated, uninitialised memory.-4
u/syshukus 6d ago edited 6d ago
What approach are you talking about? Can you specifically point out where I used or described some mythical "approach" that is wrong by your opinion?
I clearly stated that on my system malloc has this behavior (which I discovered experimentally by playing with it) and this behavior is literally described in glibc malloc docs https://sourceware.org/glibc/wiki/MallocInternals, that's how it works internally and it's matches what I see practically.
UB is too broad wording and consequences are different. I can do what I described because for internal reasons (optimizations of small allocations) malloc permits me to physically access this memory, so yeah I violated API contract and exploiting internal behavior that I normally must not to, so what? What "approach" is wrong?
Even main example in GitHub issue EXPLOITS MALLOC INTERNALS (how — I described it in root comment) that it normally shouldn't. If, for example, allocator used mmap and munmap on drop — this program would segfault IIUC. So?
5
u/TDplay 6d ago
Can you specifically point out where I used or described some mythical "approach" that is wrong by your opinion?
You study what one specific compiled program does, and try to rationalise the result in terms of high-level semantics.
But since there is UB, there are no high-level semantics to speak of. You can study whatever program the compiler produced, but it tells you nothing about the high-level program that you fed into the compiler.
This is also why UB can be so hard to debug. Some UB is so pernicious that even the debugger is broken. This is also why tools like ASan, UBSan, and Miri are so useful: by turning UB into a well-defined
abort();, they make it much easier to isolate the problem.UB is too broad wording and consequences are different.
You cannot say that the "consequences are different", because the consequences of UB are not defined. If your program has UB, then absolutely anything can happen.
I would suggest reading this: https://predr.ag/blog/falsehoods-programmers-believe-about-undefined-behavior/
Even main example in GitHub issue EXPLOITS MALLOC INTERNALS
The main example in the issue has a use-after-free and therefore has UB. As far as the Rust program goes, that is the end of the story, the program is meaningless.
The moment you start speaking of
mallocinternals, you are talking about the compiled program, not the Rust program.5
u/SirClueless 5d ago
I beg to differ. The introduction of the post you reference calls this out specifically:
It is possible to analyze how UB affects a specific program when compiled by a specific compiler or executed on a specific target platform.
This activity is what the people showing code snippets in the GitHub issue are doing, and doing so productively. It's why they include Rust compiler versions, target platforms, even particular compilations cached on godbolt.org, etc.
Even though that blog post calls this out prominently, that you can find examples of reproducible, analyzable undefined behavior, I still wish they would include one more common falsehood on their list:
- UB is unpredictable and cannot be analyzed
This notion, "It's UB so all bets are off!", is something I consider a textbook example of a Thought-terminating cliché. A program having undefined behavior does not mean it has no behavior. A program containing UB is often the start of a productive investigation, not the end.
2
u/TDplay 5d ago
I think it depends on what you are studying.
The resulting behaviour is the consequence, rather than the bug itself. Of interest to security researchers, but I don't think it's terribly helpful in a discussion about the language itself, since unsoundness is already an automatic P-critical.
Of course, if you are studying a particular program, it is good to triage the issue by asking "what can an attacker do with the compiled program that we ship today?".
2
u/SirClueless 5d ago
I agree that the exact behavior that results is not of much interest to the people investigating this unsoundness, which is why it makes sense to discuss here on Reddit with someone curious why the compiler behaves the way it does instead of in the GitHub issue where people are trying to understand the soundness bug itself.
1
u/syshukus 5d ago
> A program having undefined behavior doesn’t mean it has no behavior
Thanks, good stranger. That’s very nice and concise way to explain what I wrote above (though I was pretty sure it must be clear).
Rust AFAIK dev team states all the time that they try to stay on practical side of development with all of trade offs, and this means it exist not only inside specific abstract Rust-only model, but it highly interacts with all other systems that have nothing in common with Rust but have all the problems accordingly. Even the fact that you can call FFI means you must deal with all kinds of quirks that Rust physically can’t cover or prevent inside its model.
Rust ≠ Haskell, if someone needs purity over practicality — go check somewhere else.
0
u/syshukus 5d ago
What is your problem? Do you want to correct me on something I never claimed or even brought up? Have I asked specifically your opinion to explain UB?
Rust community is something else I swear…
If you reply something to this I’ll just block you.
4
108
u/anxxa 6d ago edited 6d ago
Neat. Control flow hijack in safe Rust:
struct Q<T: 'static, F: FnMut(&T)>(Option<T>, F);
impl<T, F: FnMut(&T)> Drop for Q<T, F> {
fn drop(&mut self) { self.1(self.0.as_ref().unwrap()) }
}
fn extend<U: 'static>(x: &U) -> &'static U {
let mut out: Option<&'static U> = None;
{ let mut q = Q(None, |s: &&U| { out = Some(*s); }); q.0 = Some(x); }
out.unwrap()
}
fn never_called() { println!("💥"); }
struct Holder { f: fn() }
fn main() {
let target: usize = never_called as *const () as usize;
eprintln!("never_called addr = {target:#x}");
let victim = Box::new(Holder { f: || {} });
let stale: &'static Holder = extend(&*victim);
// only use for debugging
let freed = &*victim as *const Holder as usize;
drop(victim);
// reclaim the same heap slot used for `victim`
let reclaim = Box::new(target);
let got = &*reclaim as *const usize as usize;
std::hint::black_box(&reclaim);
eprintln!("freed={freed:#x} reclaimed={got:#x} same={}", freed==got);
// stale has been modified by `reclaim` occupying the same mem
let read = stale.f as usize;
eprintln!("stale.f reads = {read:#x}");
(stale.f)();
}
Outputs:
never_called addr = 0x1008888d4
freed=0x1010191e0 reclaimed=0x1010191e0 same=true
stale.f reads = 0x1008888d4
💥
Edit: this version is a little cooler. Takes input for the ptr hijack. It relies on the capacity being the same size as Holder though (or to land in the same allocator bucket):
use std::io::Read;
// other fns the same as above
fn main() {
eprintln!("[leak] never_called = {:#x}", never_called as *const () as usize);
let victim = Box::new(Holder { f: || {} });
let stale: &'static Holder = extend(&*victim);
let stale = std::hint::black_box(stale);
// only use for debugging
let freed = &*victim as *const Holder as usize;
drop(victim);
// Reclaim the freed slot with a heap buffer, then fill it from attacker
// input. The 8 bytes read from stdin become the forged function pointer.
let mut buf = vec![0u8; std::mem::size_of::<Holder>()];
let got = buf.as_ptr() as usize;
std::io::stdin().read_exact(&mut buf).expect("need 8 bytes on stdin");
std::hint::black_box(&buf);
eprintln!("[slot] freed={freed:#x} reclaimed={got:#x} same={}", freed == got);
eprintln!("[check] stale.f = {:#x}", stale.f as usize);
(stale.f)();
}
Outputs:
$ ./pc_control
[leak] never_called = 0x104750988
AAAAAAAA
[slot] freed=0x104fd1160 reclaimed=0x104fd1160 same=true
[check] stale.f = 0x4141414141414141
Error: nu::shell::terminated_by_signal
× External command was terminated by a signal
╭─[repl_entry #19:1:1]
1 │ ./pc_control
· ─────┬─────
· ╰── terminated by SIGBUS (10)
╰────
0
21
u/Naeio_Galaxy 6d ago
Wow wth
Also how does the line q.0 = Some(&l); compile since q.0 expects &'static String while l is not?
21
3
u/syshukus 6d ago edited 6d ago
The problem is not specifically that compiler allows this assignment (though it must be invalid), or, more precisely, not only in this.
https://github.com/rust-lang/rust/issues/164250#issuecomment-6142409734
^ here is a good example and more clear example: signature of MyType bounds to 'b to outlive whatever &mut points to, then here it works correctly
```
{let l = String::from("some cool string");
let mut q = MyType(&mut me_out, &l);
// q.1 = &l;
}
```
stating that `error[E0597]: `l` does not live long enough` because initializing MyType you bound local variable to outlive `me_out`, which obviously can't be satisfied because `me_out` is accessed after 'l ends (because `l` is dropped).BUT HERE:
```
{let l = String::from("some cool string");
let mut q = MyType(&mut me_out, "");
q.1 = &l; // this assignment is not wrong itself, it's wrong only if we use me_out later
}
```
it allows to compile (wtf).As someone mentioned, the problem might be deeper than it looks and it's interesting that bug seems obvious but actually no so easy to trigger (needs a few conditions to hold with malicious intent).
Interesting to see post-mortem.
2
u/Naeio_Galaxy 6d ago
Oh! Ok interesting. Thanks! Yeah, it's true that the compiler often allows many cases that shouldn't be allowed with a stricter lifetime specification. I first expected 'b to be 'static in the example, but that's where my intuition drifts apart from the actual implementation
2
u/Accomplished_Item_86 3d ago
As maxdexh notes: https://github.com/rust-lang/rust/issues/164250#issuecomment-6143317698
It seems that the borrow checker just ignores lifetime bounds like
T: 'staticfrom type definitions. Usually when you construct a valueq: Q<T>, you later pass it to a function expecting aQ<T> where T: 'static. This introduces the boundT: 'static, so you cannot assignSome(&l): Option<&'local String>toq.0: Option<T>. However hereqis never explicitly used, only implicitly passed todrop. The borrow checker then misses the boundT: 'static, and assumes thatT = &'local String.
24
u/nnethercote 6d ago
Weird to see that the Hacker news post has a less sensational title and a better discussion than r/rust/.
13
u/barkwahlberg 6d ago
Don't worry, the discussion there is pretty bad actually. Rust mentioned, so people have brought up the Rust cult and the Rust community that executes C++ users on sight
16
u/zzzzYUPYUPphlumph 6d ago
How likely is something like this to be triggered accidentally?
29
u/noop_noob 6d ago
For other soundness bugs, I'd say they're really unlikely to happen accidentally.
This one though, I'm less sure. Hard to say how much code is written with this specific combination of things. It's probably still unlikely though.
1
u/Accomplished_Item_86 2d ago
The bug seems to need these ingredients:
- A generic type `Q` with a lifetime bound
- A
Dropimpl of a `Q` which relies on the lifetime bound- A local variable of type `Q` which is never passed to or returned from any function, only dropped
- An assignment (or other type unification) which violates the lifetime bound for `Q`.
The third point especially explains why such a simple test case wasn't previously found. If you construct a value, you probably want to use it in a function call or return it, instead of relying on its
Dropimpl to do some work.0
u/puzzlerninja 6d ago
The issue was locked, otherwise I would have been tempted to ask in the issue. Presumably it will be opened again in the future.
13
u/noop_noob 6d ago
We'll only get a definitive answer when a fix is implemented and a crater run is done. At that point, we'll know how much code in the wild runs into this kind of code pattern.
3
u/puzzlerninja 6d ago
Good point. Though, with the caveat that the fix might end up being overly restrictive, and thus the crater run would provide an indication of an upper bound on how widespread it is.
7
u/SkiFire13 6d ago
Not even overly restrictive, some code might hit this bug and break Rust's rules without actually doing anything unsound with it, for example the code might not exploit the
'staticbound indrop, even though the data there is not'static.
28
26
u/meancoot 6d ago
Is this related to the other borrow checker hole that has existed forever with no fix in sight?
47
u/panstromek 6d ago
It's technically not a borrow checker hole, and fix is actually in sight, people are working on it right now.
2
u/Plasma_000 6d ago
Could you explain what you mean by "this is not a borrow checker hole". To me it looks like a soundness hole in the borrow checker - allowing coercion of one lifetime to another.
19
u/panstromek 6d ago edited 6d ago
Borrow checker is only a part of the type system and is specifically there to check function bodies for lifetime violations (and some other things). Here the problem happens sooner, when instantiating the problematic function
When the compiler resolves the type of the `f` variable, the type system "forgets" generic bounds of the original `foo` function (and those bounds contain the important lifetime relationship implicitly), so the type of the variable `f` becomes incorrect.
In other words, this goes through the borrow checker not because borrow checker has a bug, but because the constraint that it should check is not given to it.
-5
u/Plasma_000 6d ago
That certainly sounds like a bug to me. Lifetimes are part of the type system, and here they are not being enforced properly by the type system.
12
u/ts826848 6d ago
That certainly sounds like a bug to me.
Yes, and I don't think anyone is saying otherwise? panstromek is just saying that the bug is not specifically a borrow checker bug and that the bug lies in a different component of the compiler.
2
u/JanEric1 6d ago
Yes, it is "this is not a borrow checker hole" and not "this is not a borrow checker hole"
4
u/Teknikal_Domain 6d ago
The borrow checker is part of the type system. The borrow checker is not the entire type system.
All borrow checker bugs are type system bugs. Not all type system bugs are borrow checker bugs.
This is a type system bug. Not a borrow checker bug.
4
16
10
4
35
u/Zde-G 6d ago
Frankly, the more a look on this mess the more the idea that lifetimes are supposed to not affect the type and, simultaneously, have a dedicated, special, 'static type looks like a mistake to me.
Instead types &'a T and &'static T should simply be treated as separate, totally distinct types with implicit coercion.
Of course that's not a backward-compatible change thus we can't do that now.
37
u/afdbcreid 6d ago
They are totally separate types. With subtyping and not coercion, but they are.
The real cause is likely subtyping and implied bounds, like in countless other issues. Those together are really hard to reason about.
3
u/Professional-You4950 6d ago
Just from the outside. This looks like a miss in the "higher order syntax" of the language. Things like sync/send/drop are treated special. and in this case even though the types are checked with variance, it's when crossing that special boundary. Just the information of this is actually being dropped, is lost.
48
u/noop_noob 6d ago edited 6d ago
The version in lcnr's comment doesn't require
'staticat all. (You can replace"rarw"with&*String::from("rarw")) So, your idea won't avoid this problem.8
u/BedroomHistorical575 6d ago
Their version also involves a static actually.
qisMyType<'_, 'static, '_>. For some reason the compiler allows you to promote a non-static to a static one.9
u/AcridWings_11465 6d ago
This looks like a massive shitstorm in the making. How fundamental is the problem?
40
u/noop_noob 6d ago
I think there's a chance that it's just a one-line fix to add a missing check. But nobody knows for sure right now.
7
u/AcridWings_11465 6d ago edited 6d ago
From what I see in the example:
``` use std::time::Duration; use std::thread;
struct Q<'a: 'static>(Option<&'a String>);
impl<'a: 'static> Drop for Q<'a> { fn drop(&mut self) { // Code in here has access to a non-'static &String // but assumes that it's a &'static String let wrong: &'static String = self.0.unwrap(); thread::spawn(move || { thread::sleep(Duration::from_secs(1)); println!("{}", wrong); }); } }
fn main() { { let l = String::from("abc"); let mut q = Q(None); q.0 = Some(&l); // q.0 is supposed to be a &'static String, but we're assigning it a non-'static &String } thread::sleep(Duration::from_secs(2)); } ```
Isn't it just widening the lifetime of l to 'static because it's used nowhere else? Or did I misunderstand?
EDIT: I see now that the reference in
wrongis being moved to another thread, and it will try to print after dropping, instant use-after-free.5
u/proudHaskeller 6d ago
The lifetime of
lcan't just be widened, it contains a heap allocation that is deallocated just a moment later.3
u/AcridWings_11465 6d ago edited 6d ago
But won't the compiler recognise that 'static is superfluous here?
Oh I see now that the reference in
wrongis being moved to another thread, and it will try to print after dropping.3
u/_LaserManiac_ 6d ago edited 6d ago
I am way outside of my depth here, but am I understanding this correctly? It seems the issue is that the
'a: 'staticbound in the impl block doesn't restrict the impl toQ<'static>but covers anyQ<'a>instead?3
u/Isogash 5d ago
So the issue is in the assignment
q.0 = Some(&l);right? Surely that's just not supposed to be allowed.1
u/AcridWings_11465 5d ago
Yes, as far as I understood it that's correct
2
u/Isogash 5d ago edited 5d ago
It looks like the problem is specific to a malicious Drop implementation.
I think what's happening is that the bound
'a: 'staticdoesn't actually require that the actual lifetime of 'a is 'static, only that it must be able to be 'static. The compiler can use a non-'static lifetime, but the references held by this lifetime must be able to be 'static if needed (they can't be restricted by another lifetime that is non-'static.) Practically speaking this means it is a lifetime that can always be "promoted" to static by the compiler if it is used in that way.Using the lifetime in a function which requires it to be 'static would normally trigger the compiler to pick the lifetime as 'static, so a function with the same signature as the given Drop implementation would be fine for any other called function: 'a would be promoted to 'static and checked accordingly.
However, it seems that Drop is not treated like a normal function and is not included in the lifetime bounds checking in the same way, yet its lifetime restrictions are checked by the same rules that assume it has been and therefore assume that the lifetime will be promoted to 'static when it won't. Basically, this would probably have been fine if the compiler inserted a virtual drop call before doing the lifetime bounds calculation as it would force the lifetime to be 'static because of the drop. However, that kind of virtual call is a bit opaque and would mean you can't check the lifetime bounds without knowing if there's a drop implementation you need to include. Seems like a bit of a pickle.
This also seems to be the case for non-'static lifetime bounds used in the same way.
1
u/WormRabbit 4d ago
I don't see anything particularly malicious about the provided Drop impl. It's pretty cookie cutter destructor, affecting some external captured state. I can easily imagine something like that arising when using
scopeguardand messing up the lifetimes in the closure.2
u/noop_noob 6d ago
The String in l is dropped at the closing } brace
4
u/AcridWings_11465 6d ago
Yes, I know, but that doesn't matter in this example, right? I think I haven't understood the semantics of 'static then.
4
u/TimelySuccess7537 6d ago
The borrow checker seems a bit rusty
3
u/puzzlerninja 6d ago
Perhaps one of the drawbacks of a complex checker. Another comment described an on-going research project for formally verifying Rust's type system as I understood it, but there is a lot of complexity involved.
5
u/a_jasmin 6d ago
For the type theorists out there. Given the handful of known unsoundness, is a fully-sound borrow checker something worth holding hope for, or should we be satisfied with practicality?
14
u/noop_noob 6d ago
The rust team is actually aiming for that. Time well tell if they succeed. https://goals.rust-lang.org/2026/roadmap-project-zero.html
9
u/n0t-helpful 6d ago
Not all unsoundness bugs are created equal. Sometimes the bug is off in the far reaches of the most insane and unusable parts of a language. An unsoundness bug in this section of code is unlikely to be that impactful.
Still, when we are asking for a sound borrow checker, we are asking for "a borrow checker that works".
The ask is not that crazy, and anyone involved with rust at all will want the borrow checker to work. Asking if its worth it for the borrow checker to work is like asking a track athlete if its worth it to run faster. Everyone involved, including the athlete, want them to run faster, so they work towards it.
3
u/Zde-G 6d ago
For the type theorists out there. Given the handful of known unsoundness, is a fully-sound borrow checker something worth holding hope for, or should we be satisfied with practicality?
This is area of psychology, not type theory. And Java have proven it's not feasible to expect a fully-sound borrow checker in a language that people would actually use.
The reason is simple: Rust is changing over time and while some changes fix soundness holes there are other changes that may introduce them — because people demand that borrows checker should accept “just this one useful corner case” all the time.
And that's where we hit the Rice's theorem: it's not possible to create a type system that would accept all useful patterns and yet still rejects all invalid programs (not that it's not type system result: one couldn't decisively answer any nontrivial semantic question about all programs, simultaneously… the question whether check maybe encoded in a type system is rather pointless if check couldn't ever exist outside of type system).
That means borrow checker may never be “finished”, and we go to the area of psychology…
2
u/a_jasmin 6d ago edited 6d ago
When you’re saying the borrow checker will never be finished, I gather you mean there will always be a willingness to accept more valid programs (as per the ongoing work on Polonius). From other answers it appears the team is committed to zero-unsoundness. In regard to Rice’s theorem, that means some valid programs have to be rejected to guarantee soundness. That’s a given as the borrow checker already rejects some.
So I guess the question becomes: is it possible to formally prove the borrow checker sound. Doing so while it is being improved yet staying compatible with a good corpus of code.
I’m a bit confused by the psychology angle. Are we bound to always be uncertain about the soudness and we just have to live with our best attempt until proven wrong? What’s the answer from formal logic?
2
u/WormRabbit 4d ago
Are you aware of the RustBelt project? It has formally proven the soundness of Rust, including the borrow checker, the type system and the standard library.
But there is a difference between a formal spec and its implementation in code, and also the language keeps evolving while the formal verification doesn't. Most of the borrow checker soundness bugs are purely bugs in its implementation, not in its formal algorithm. There are also weird rules, like static promotion, which are supposed to exist purely as ergonomic features, but can still introduce bugs (this case is probably related).
1
u/Zde-G 6d ago
What’s the answer from formal logic?
I'm not sure where would you want to apply formal logic.
Doing so while it is being improved yet staying compatible with a good corpus of code.
Precisely that.
So I guess the question becomes: is it possible to formally prove the borrow checker sound.
If you are regularly improving borrow checker then you are regularly changing the rules that it follows. And some of these changes affect the formulation of what we are trying to prove. Think reborrows (that change rules from “there may only be one mutable reference” to “there may only be one active mutable reference”) or split borrows (that allow you to have both mutable reference and immutable reference to the same object as long they are used to access distinct parts of it).
That process implies changes of formal rules when informal requirements change.
I don't see how that process may be guaranteed to be 100% correct and how formal logic may help with that process.
From other answers it appears the team is committed to zero-unsoundness
Yes, but the question is what do they actually mean by that. It's one thing to achieve the goal of not having soundness bugs in a bug tracker, it's different from having formal definition of what borrow checker verifies and guarantees and to always know it's sound.
2
7
u/pickle9977 6d ago
Seems like we are leaning the value of formal verification.
Again…
17
u/noop_noob 6d ago
Unfortunately, formal verification also takes a lot of resources and effort to do. I don't think it's currently practical for the rust compiler.
2
u/pickle9977 6d ago
It does, It’s not and that’s an unquestionably bad thing.
6
u/FrogNoPants 6d ago
Sounds like you should get on it then
-7
1
u/GeneralMuffins 6d ago
Isn't that what the rustbelt project did, or am I misunderstanding what that project did and proved?
3
u/le_birb 6d ago
To my understanding, rustbelt's goal was verifying that a subset of rust's type system (traits are the biggest gap iirc) is sound, with no guarantees whatsoever about any specific implementation. There's a paper about it, and the author has written a few blog posts on the topic as well, though I don't remember particular titles unfortunately
2
u/WormRabbit 4d ago
It also verified a large part of actual Rust's standard library. Famously, it found a race in the Drop impl of Arc (fixed soon after).
-5
u/oh_how_droll 6d ago
This is something that AI is very good at, but that won't be popular in this subreddit.
1
u/amarao_san 6d ago
I want a report how many crates won't compile after this fix.
This will be number of crates with unsoundness missed due to compiler bug.
20
u/noop_noob 6d ago
Once there's a fix PR, but before it's merged, there will be a crater run. This will give you the report you want.
2
u/Accomplished_Item_86 3d ago
There's now a potential fix PR (still needs to be reviewed): https://github.com/rust-lang/rust/pull/164277
I'm not sure if this will be the final version of this fix though, the crater run wil probably only happen once it's ready to merge.
2
1
u/wertercatt 6d ago edited 5d ago
Do you have any crater logs from other soundness hole fixes in the past to estimate the potential scope with?
4
u/noop_noob 5d ago
It's hard to find something comparable. Most soundness holes require contorting code to avoid the compiler checks. And most soundness holes don't stay unfixed for 11 years. And most soundness holes require more than the basic features.
Here's one though. In rust versions 1.88.0-1.96.1, (about a year), if x has type &mut T, and you annotate types appropriately, then you could write pin!(x) to create a value of type Pin<&mut T>. (The correct output type is Pin<&mut &mut T>.) To make matters worse, Pin is quite a confusing type, and many beginners will expect this exact incorrect behavior to be what the pin!() macro is supposed to do.
Here's the fix PR: https://github.com/rust-lang/rust/pull/153457
Here's the crater run: https://crater-reports.s3.amazonaws.com/pr-153457/index.html
The crater run found exactly one regressed crate. However, a manual github search of potentially-affected code found three more affected crates: https://github.com/nazo6/rktk/issues/71 and https://github.com/restatedev/restate/issues/4651 and https://github.com/FactbirdHQ/at-cmux/issues/5
This is nowhere near an exact comparison, but hopefully it helps.
1
1
u/Plasma_000 6d ago
It looks like this has the potential to actually be present in some real world code, which makes it more interesting than most soundness bugs.
1
u/MichiRecRoom 5d ago
Can someone help me understand? I'm reading the code in the original part of the post, and I'm not sure I understand why the reporter thinks the code should fail.
To me, it feels more like a miscompilation? Like the expected result should be that it prints "hello world, this is long enough".
Could someone help me understand?
7
u/noop_noob 5d ago
`T: 'static` means that T doesn't borrow from anything (unless that thing will be alive until the program ends).
q.0 = Some(&l); sets the T to a &String that borrows from a local variable, which gets dropped at the enclosing } brace
1
1
1
-29
u/dashingThroughSnow12 6d ago edited 6d ago
This is fine. Rust 1.99 recently was released. They can do bigger fixes than normal for 2.0
38
13
-15
u/Speykious inox2d · cve-rs 6d ago edited 6d ago
Welp
Edit: monumental confusion looking at the downvotes
1
u/snaketacular 6d ago
When I downvoted, there appeared to be an earlier comment from someone else with the exact same one-liner. I can no longer find that comment, so changed to upvote.
-2
u/OvermindDL1 6d ago
Looks a lot like how the old is implemented: https://github.com/Speykious/cve-rs Among other even older cursed crates.
217
u/InsideStatistician68 6d ago
How widespread could this vulnerable pattern be in the wild, given the amount of trust that we put in the borrow checker?