denoland / rusty_v8

Rust bindings for the V8 JavaScript engine
https://crates.io/crates/v8
MIT License
3.28k stars 308 forks source link

V8 Sandbox feature #1448

Open mlafeldt opened 6 months ago

mlafeldt commented 6 months ago

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:

https://github.com/denoland/rusty_v8/blob/89fbf2a0511d9a5d6da69686543db1f021feb980/.gn#L33

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 commented 6 months ago

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 commented 6 months ago

@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 commented 4 months ago

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 commented 4 months ago

We landed 12.6-lgkr in rusty_v8 which was a prereq for pointer compression: https://github.com/denoland/rusty_v8/pull/1473

devsnek commented 3 months ago

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

wingo commented 3 months ago

https://chromium-review.googlesource.com/c/v8/v8/+/5632279

devsnek commented 3 months ago

@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 commented 3 months ago

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.