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

V8 Sandbox feature #1448

Open
mlafeldt opened this issue Apr 4, 2024 · 8 comments
Open

V8 Sandbox feature #1448

mlafeldt opened this issue Apr 4, 2024 · 8 comments

Comments

@mlafeldt
Copy link

mlafeldt commented Apr 4, 2024

After reading https://v8.dev/blog/sandbox, I wonder whether you plan to enable the sandbox feature anytime soon?

The V8 Sandbox is a new security mechanism designed to prevent memory corruption in V8 from impacting other memory in the process. The sandbox is motivated by the fact that current memory safety technologies are largely inapplicable to optimizing JavaScript engines. While these technologies fail to prevent memory corruption in V8 itself, they can in fact protect the V8 Sandbox attack surface. The sandbox is therefore a necessary step towards memory safety.

It's currently disabled:

rusty_v8/.gn

Line 33 in 89fbf2a

v8_enable_sandbox = false

And needs to be enabled at build time:

The V8 Sandbox must be enabled/disabled at build time using the v8_enable_sandbox build flag. It is (for technical reasons) not possible to enable/disable the sandbox at runtime. The V8 Sandbox requires a 64-bit system as it needs to reserve a large amount of virtual address space, currently one terabyte.

The V8 Sandbox has already been enabled by default on 64-bit (specifically x64 and arm64) versions of Chrome on Android, ChromeOS, Linux, macOS, and Windows for roughly the last two years. Even though the sandbox was (and still is) not feature complete, this was mainly done to ensure that it does not cause stability issues and to collect real-world performance statistics. Consequently, recent V8 exploits already had to work their way past the sandbox, providing helpful early feedback on its security properties.

Thanks!

@mlafeldt
Copy link
Author

mlafeldt commented Apr 4, 2024

I already found one complication: v8_enable_sandbox requires v8_enable_pointer_compression, which was disabled in #1302 due to a mem leak.

diff --git .gn .gn
index f7097dd..0c1f746 100644
--- .gn
+++ .gn
@@ -30,7 +30,7 @@ default_args = {
   symbol_level = 1
   use_debug_fission = false
 
-  v8_enable_sandbox = false
+  v8_enable_sandbox = true
   v8_enable_javascript_promise_hooks = true
   v8_promise_internal_field_count = 1
   v8_use_external_startup_data = false
@@ -63,7 +63,7 @@ default_args = {
   # be cleaned which causes resource exhaustion. Disabling pointer compression
   # makes sure that the EPT is not used.
   # https://bugs.chromium.org/p/v8/issues/detail?id=13640&q=garbage%20collection&can=2
-  v8_enable_pointer_compression = false
+  v8_enable_pointer_compression = true
 
   # Maglev *should* be supported when pointer compression is disabled as per
   # https://chromium-review.googlesource.com/c/v8/v8/+/4753150, but it still

@ry
Copy link
Member

ry commented Apr 5, 2024

@wingo is working on a fix for the memory leak, which will hopefully allow us to re-enabling pointer compression.
ref
https://bugs.chromium.org/p/v8/issues/detail?id=13640
https://chromium-review.googlesource.com/c/v8/v8/+/5185345

@ry
Copy link
Member

ry commented May 14, 2024

Fix has been completed. Andy wrote a blog post about it https://wingolog.org/archives/2024/05/13/partitioning-pitfalls-for-generational-collectors

We're waiting for a new V8 LKGR before re-enabling pointer compression.

@mmastrac
Copy link
Contributor

We landed 12.6-lgkr in rusty_v8 which was a prereq for pointer compression: #1473

@devsnek
Copy link
Member

devsnek commented Jul 2, 2024

It looks like sandbox now requires shared read only space as well.

@wingo
Copy link

wingo commented Jul 3, 2024

@devsnek
Copy link
Member

devsnek commented Jul 3, 2024

@wingo Am I understanding correctly here that ReadOnlyHeap and SharedReadOnlyHeap are both being kind of "merged" into the sharedness of IsolateGroup? That would be great if we can control isolate grouping in the public API as the only reason we don't use SharedReadOnlyHeap right now is that we have different isolate snapshots running in the same process. If we could put those in separate groups that would be great.

@wingo
Copy link

wingo commented Jul 4, 2024

Roughly speaking, an isolate group has a one-to-one relationship with a pointer cage. That pointer cage may be sandboxed or not, depending on the configuration. If shared read-only heaps are enabled, they are associated with the isolate group / pointer-cage. There is still no public API for isolate groups; we have some drafts but we want to avoid shipping something that doesn't match use-cases.

Right now we are most concerned with the use case where all isolate groups have the same snapshot. I think our current drafts should work with different snapshots though also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants