It's interesting to browse the large collection under chrome/browser/ui and ponder over how many of them are use-after-free for data where the performance of manual memory management really doesn't matter. Like [1] which is around the lifecycle of a "choose a file" dialog.
It feels like in the big picture sense it would be better to just always using some sort of smarter/slower pointers in this kind of code just for extra defense. I saw in [2] there is some sort of `raw_ptr<T>` type [3] that seems to intend to help, so maybe the crash in [2] was actually successfully defended against?
It's too bad that there's not a good way to have a broader way to switch between dialects within a project where in one place it's "this portion of the code is perf-critical and carefully reviewed" and in another "this portion of the code is perf-oblivious and has lots of async state that's easy to get wrong". I've wondered if it's almost worth mixing two separate languages (like a GCed one for the latter) just to make the distinction clear.
[Disclaimer: worked on this code many years ago, wouldn't be surprised if I caused >0 of these bugs...]
> It's too bad that there's not a good way to have a broader way to switch between dialects within a project where in one place it's "this portion of the code is perf-critical and carefully reviewed" and in another "this portion of the code is perf-oblivious and has lots of async state that's easy to get wrong". I've wondered if it's almost worth mixing two separate languages (like a GCed one for the latter) just to make the distinction clear.
You are describing Rust's "unsafe" keyword.
And this kind of code is very literally the original impetus of it. The language was sort of originally designed to implement a browser, after all.
I have written a lot of Rust, and I am describing something different. Rust lets you write code that cares a lot about memory (with lifetimes etc.) with either compiler assistance or without. What I am describing is code that doesn't much care about memory and would be happy to sacrifice performance for it.
The use of unsafe in systems programming languages at very least goes all the way back to ESPOL and NEWP in 1961, with many other cases in between until Rust came to be.
> I've wondered if it's almost worth mixing two separate languages (like a GCed one for the latter) just to make the distinction clear.
Python, with perf critical sections written in C or Rust is pretty much that. From what I hear the Rust-Python bindings are especially good, and make the correctness part easier even in the performance critical parts.
Or you can go the opposite way and call out to a scripting language from your fast language. Today everyone is hyped about wasm for that, but we also have about two decades of computer games using lua for that (and games are probably the single biggest category of performance sensitive software)
I had wanted to use Oilpan GC in the browser process for this reason, but the browser side folks were very opposed to using the blink libraries back then.
Most of the Chrome UI code is written with Web UI at least. These days I think they should consider typescript for more orchestration things within the browser too. It's a proven strategy by Electron.
I think the momentum is actually around MiraclePtr now though (as you found).
If I understand the blog posts related to adopting Rust into Chrome, it appears they finally accepted that Oilpan GC and MiraclePtr aren't enough, as originally argued for.
The "we are not going with Rust" blog post, in 2021,
Oilpan isn't without issues though: finalization causes quite a few headaches, implementation details like concurrent marking make it hard to support things like std::variant, and the interface between Oilpan and non-Oilpan types often introduces opportunities for unsafety.
Indeed. It's tradeoffs, but they've been sufficient for much of the codebase for a very long time. Taking no major action (Oilpan or memory safe language) for nearly a decade was also a tradeoff. I don't think the long list of security issues there was worth it.
Hmm. The blink dirs seem to have a very large number of security issues relevant too, despite oilpan. Which is what we'd expect, honestly; oilpan does not solve all uaf problems, uafs are not all (or even the majority of) security problems, etc. The combo of "sufficient for much of the codebase" and "the long list of security issues" paints a picture of most of the codebase being secure due to oilpan, while UI code is riddled with holes due to its lack. The reality is dramatically more nuanced (as you know, but readers of your comment might not).
As a views maintainer, I'm familiar with some of the security bugs on the UI side. Clickjacking-type issues are more common there than uafs. Uafs are an issue, but the problems are not so much due to problematic use of unsafe c++ types and idioms as problematic API designs -- resulting in, for example, cases where it's not clear whether an object is expected to be able to respond safely to calls at all, whether or not it's technically alive. Oilpan, MiraclePtr, proper use of smart pointers would all help bandaid the uafs, but are in many cases difficult to apply correctly without understanding (and often fixing) the underlying systemic design problems. Which is happening, but slowly.
There are also more dimensions of tradeoffs involved, but this is long winded enough as it is. The tldr is that at this point I would consider a couple other options better uses of effort for tackling this specific problem compared to converting browser types to oilpan.
Possibly another way of expressing your point: after writing my above comment, I found myself wondering how much a system that kept these UAF pointers alive longer (to eliminate the UAFs, e.g a GC) would actually reduce the attack surface -- the UAF-ish bugs are still bugs, and code poking at a GC-preserved object that the rest of the code doesn't really expect to still be alive might itself be pretty fraught.
> the UAF-ish bugs are still bugs, and code poking at a GC-preserved object that the rest of the code doesn't really expect to still be alive might itself be pretty fraugh
For the LayoutObject heirarchy - the team doing that conversion added a NOT_DESTROYED() macro for this reason. It's gross, but was the least worst option.
As an aside - the performance of oilpan is broadly net positive now if you avoid some of the pitfalls. (The largest being a write into a Member<> requires a write-barrier). E.g. Things become trivially destructible, and no ref incrementing/decrementing, etc.
It might be better to say raw_ptr<T> is a dumb pointer wrapper. raw_ptr is discouraged in favor of unique_ptr/WeakPtr/RefCounted; but it is used in places where you'd have a T* at rest.
It feels like in the big picture sense it would be better to just always using some sort of smarter/slower pointers in this kind of code just for extra defense. I saw in [2] there is some sort of `raw_ptr<T>` type [3] that seems to intend to help, so maybe the crash in [2] was actually successfully defended against?
It's too bad that there's not a good way to have a broader way to switch between dialects within a project where in one place it's "this portion of the code is perf-critical and carefully reviewed" and in another "this portion of the code is perf-oblivious and has lots of async state that's easy to get wrong". I've wondered if it's almost worth mixing two separate languages (like a GCed one for the latter) just to make the distinction clear.
[Disclaimer: worked on this code many years ago, wouldn't be surprised if I caused >0 of these bugs...]
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=120103... [2] https://bugs.chromium.org/p/chromium/issues/detail?id=132323... [3] https://source.chromium.org/chromium/chromium/src/+/main:bas...