src: refactor sandbox code into helper

Author: codebytere

src/api/environment.cc

@@ -305,11 +305,15 @@ void SetIsolateUpForNode(v8::Isolate* isolate) {
 
 IsolateGroup GetOrCreateIsolateGroup() {
 #ifndef V8_ENABLE_SANDBOX
-  // When the V8 sandbox is enabled, all isolates must share the same sandbox
-  // so that ArrayBuffer backing stores allocated via NewDefaultAllocator()
-  // (which uses the default IsolateGroup's sandbox) are valid for all
-  // isolates. Creating new groups would give each group its own sandbox,
-  // causing a mismatch with the allocator.
+  // IsolateGroup::CanCreateNewGroups() only reflects whether multiple pointer
+  // compression cages are supported at build time; it has no awareness of the
+  // V8 sandbox (they are orthogonal concepts). However, when the sandbox is
+  // enabled the cage-allocation helpers (node_v8_sandbox.h) route through the
+  // default allocator, which allocates inside the *default* group's sandbox.
+  // Creating a new group would give it its own sandbox, so ArrayBuffers
+  // allocated by the default allocator would be invalid for isolates in that
+  // group. We therefore always fall through to GetDefault() when the sandbox
+  // is enabled.
   if (IsolateGroup::CanCreateNewGroups()) {
     return IsolateGroup::Create();
   }

src/crypto/crypto_dh.cc

@@ -7,6 +7,7 @@
 #include "memory_tracker-inl.h"
 #include "ncrypto.h"
 #include "node_errors.h"
+#include "node_v8_sandbox.h"
 #ifndef OPENSSL_IS_BORINGSSL
 #include "openssl/bnerr.h"
 #endif
@@ -61,18 +62,11 @@ MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
     bool secure;
   };
 #ifdef V8_ENABLE_SANDBOX
-  auto backing = ArrayBuffer::NewBackingStore(
-      env->isolate(),
-      data.size(),
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
+  auto backing = CopyCageBackingStore(data.get(), data.size());
   if (!backing) {
     THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
     return MaybeLocal<Value>();
   }
-  if (data.size() > 0) {
-    memcpy(backing->Data(), data.get(), data.size());
-  }
 #else
   auto backing = ArrayBuffer::NewBackingStore(
       data.get(),

src/crypto/crypto_util.cc

@@ -7,6 +7,7 @@
 #include "ncrypto.h"
 #include "node_buffer.h"
 #include "node_options-inl.h"
+#include "node_v8_sandbox.h"
 #include "string_bytes.h"
 #include "threadpoolwork-inl.h"
 #include "util-inl.h"
@@ -364,22 +365,12 @@ std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore(
   // only if size_ is zero.
   CHECK_IMPLIES(size_ > 0, allocated_data_ != nullptr);
 #ifdef V8_ENABLE_SANDBOX
-  // If the v8 sandbox is enabled, then all array buffers must be allocated
-  // via the isolate. External buffers are not allowed. So, instead of wrapping
-  // the allocated data we'll copy it instead.
-
-  // TODO(@jasnell): It would be nice to use an abstracted utility to do this
-  // branch instead of duplicating the V8_ENABLE_SANDBOX check each time.
-  std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
-      env->isolate(),
-      size(),
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
+  std::unique_ptr<BackingStore> ptr = CopyCageBackingStore(
+      allocated_data_, size());
   if (!ptr) {
     THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
     return nullptr;
   }
-  memcpy(ptr->Data(), allocated_data_, size());
   OPENSSL_clear_free(allocated_data_, size_);
 #else
   std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
@@ -690,20 +681,18 @@ namespace {
 // using OPENSSL_malloc. However, if the secure heap is
 // initialized, SecureBuffer will automatically use it.
 void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-#ifdef V8_ENABLE_SANDBOX
-  // The v8 sandbox is enabled, so we cannot use the secure heap because
-  // the sandbox requires that all array buffers be allocated via the isolate.
-  // That is fundamentally incompatible with the secure heap which allocates
-  // in openssl's secure heap area. Instead we'll just throw an error here.
-  //
-  // That said, we really shouldn't get here in the first place since the
-  // option to enable the secure heap is only available when the sandbox
-  // is disabled.
-  UNREACHABLE();
-#else
   CHECK(args[0]->IsUint32());
   uint32_t len = args[0].As<Uint32>()->Value();
+#ifdef V8_ENABLE_SANDBOX
+  // The V8 sandbox requires all array buffers to be allocated inside the cage,
+  // which is incompatible with OpenSSL's secure heap. Fall back to a regular
+  // V8-managed allocation. The --secure-heap option is disabled at the CLI
+  // level when the sandbox is enabled, but this function can still be reached
+  // via internal callers like randomUUID.
+  Local<ArrayBuffer> buffer = ArrayBuffer::New(args.GetIsolate(), len);
+  args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len));
+#else
+  Environment* env = Environment::GetCurrent(args);
 
   auto data = DataPointer::SecureAlloc(len);
   CHECK(data.isSecure());

src/crypto/crypto_x509.cc

@@ -7,6 +7,7 @@
 #include "memory_tracker-inl.h"
 #include "ncrypto.h"
 #include "node_errors.h"
+#include "node_v8_sandbox.h"
 #include "util-inl.h"
 #include "v8.h"
 
@@ -141,19 +142,11 @@ MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer* bio) {
   if (!mem) [[unlikely]]
     return {};
 #ifdef V8_ENABLE_SANDBOX
-  // If the v8 sandbox is enabled, then all array buffers must be allocated
-  // via the isolate. External buffers are not allowed. So, instead of wrapping
-  // the BIOPointer we'll copy it instead.
-  auto backing = ArrayBuffer::NewBackingStore(
-      env->isolate(),
-      mem->length,
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
+  auto backing = CopyCageBackingStore(mem->data, mem->length);
   if (!backing) {
     THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
     return MaybeLocal<Value>();
   }
-  memcpy(backing->Data(), mem->data, mem->length);
 #else
   auto backing = ArrayBuffer::NewBackingStore(
       mem->data,

src/env.cc

@@ -642,7 +642,23 @@ void IsolateData::MemoryInfo(MemoryTracker* tracker) const {
   // TODO(joyeecheung): implement MemoryRetainer in the option classes.
 }
 
+void Environment::AddTraceCategoryMapping(const uint8_t* source,
+                                          uint8_t* dest) {
+  trace_category_mappings_.push_back({source, dest});
+}
+
+void Environment::SyncTraceCategoryBuffers() {
+  for (const auto& mapping : trace_category_mappings_) {
+    *mapping.dest = *mapping.source;
+  }
+}
+
 void TrackingTraceStateObserver::UpdateTraceCategoryState() {
+  // Sync sandbox-copied trace category buffers before anything else.
+  // This must happen regardless of thread/JS-callability checks below,
+  // because the underlying enabled_pointer values have already changed.
+  env_->SyncTraceCategoryBuffers();
+
   if (!env_->owns_process_state() || !env_->can_call_into_js()) {
     // Ideally, we’d have a consistent story that treats all threads/Environment
     // instances equally here. However, tracing is essentially global, and this

src/env.h

@@ -472,6 +472,15 @@ class TickInfo : public MemoryRetainer {
   AliasedUint8Array fields_;
 };
 
+// Mapping between a trace category enabled pointer (owned by the trace
+// infrastructure, outside the V8 sandbox) and a cage-allocated copy that
+// backs a JS Uint8Array. Used to keep the two in sync when V8_ENABLE_SANDBOX
+// prevents wrapping the external pointer directly.
+struct TraceCategoryMapping {
+  const uint8_t* source;  // Original enabled_pointer (external, read-only).
+  uint8_t* dest;          // Cage-allocated copy backing the JS buffer.
+};
+
 class TrackingTraceStateObserver :
     public v8::TracingController::TraceStateObserver {
  public:
@@ -731,6 +740,9 @@ class Environment final : public MemoryRetainer {
   void PrintSyncTrace() const;
   inline void set_trace_sync_io(bool value);
 
+  void AddTraceCategoryMapping(const uint8_t* source, uint8_t* dest);
+  void SyncTraceCategoryBuffers();
+
   inline void set_force_context_aware(bool value);
   inline bool force_context_aware() const;
 
@@ -1162,6 +1174,7 @@ class Environment final : public MemoryRetainer {
   int should_not_abort_scope_counter_ = 0;
 
   std::unique_ptr<TrackingTraceStateObserver> trace_state_observer_;
+  std::vector<TraceCategoryMapping> trace_category_mappings_;
 
   AliasedInt32Array stream_base_state_;
 

src/node_buffer.cc

@@ -32,6 +32,7 @@
 #include "simdutf.h"
 #include "string_bytes.h"
 
+#include "node_v8_sandbox.h"
 #include "util-inl.h"
 #include "v8-fast-api-calls.h"
 #include "v8.h"
@@ -123,6 +124,18 @@ Local<ArrayBuffer> CallbackInfo::CreateTrackedArrayBuffer(
   CHECK_NOT_NULL(callback);
   CHECK_IMPLIES(data == nullptr, length == 0);
 
+#ifdef V8_ENABLE_SANDBOX
+  // When the V8 sandbox is enabled, external backing stores are not supported.
+  // Copy the data into the cage and immediately free the original via callback.
+  void* cage_data = SandboxAllocate(length, /* zero_fill */ false);
+  CHECK_NOT_NULL(cage_data);
+  if (length > 0) memcpy(cage_data, data, length);
+  callback(data, hint);
+  std::unique_ptr<BackingStore> bs = ArrayBuffer::NewBackingStore(
+      cage_data, length, SandboxBackingStoreDeleter, nullptr);
+  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
+  // No CallbackInfo needed — the original data has already been freed.
+#else
   CallbackInfo* self = new CallbackInfo(env, callback, data, hint);
   std::unique_ptr<BackingStore> bs =
       ArrayBuffer::NewBackingStore(data, length, [](void*, size_t, void* arg) {
@@ -140,6 +153,7 @@ Local<ArrayBuffer> CallbackInfo::CreateTrackedArrayBuffer(
     self->persistent_.Reset(env->isolate(), ab);
     self->persistent_.SetWeak();
   }
+#endif  // V8_ENABLE_SANDBOX
 
   return ab;
 }
@@ -1452,9 +1466,7 @@ inline size_t CheckNumberToSize(Local<Value> number) {
   double maxSize = static_cast<double>(std::numeric_limits<size_t>::max());
   CHECK(value >= 0 && value < maxSize);
   size_t size = static_cast<size_t>(value);
-#ifdef V8_ENABLE_SANDBOX
-  CHECK_LE(size, v8::internal::kMaxSafeBufferSizeForSandbox);
-#endif
+  CHECK_LE(size, v8::ArrayBuffer::kMaxByteLength);
   return size;
 }
 
@@ -1476,35 +1488,13 @@ void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
       env->isolate_data()->is_building_snapshot()) {
     buf = ArrayBuffer::New(isolate, size);
   } else {
-#ifdef V8_ENABLE_SANDBOX
-    std::unique_ptr<ArrayBuffer::Allocator> allocator(
-        ArrayBuffer::Allocator::NewDefaultAllocator());
-    void* data = allocator->AllocateUninitialized(size);
+    void* data = SandboxAllocate(size, /* zero_fill */ false);
     if (!data) [[unlikely]] {
       THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
       return;
     }
     std::unique_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
-        data,
-        size,
-        [](void* data, size_t length, void*) {
-          std::unique_ptr<ArrayBuffer::Allocator> allocator(
-              ArrayBuffer::Allocator::NewDefaultAllocator());
-          allocator->Free(data, length);
-        },
-        nullptr);
-#else
-    std::unique_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
-        isolate,
-        size,
-        BackingStoreInitializationMode::kUninitialized,
-        v8::BackingStoreOnFailureMode::kReturnNull);
-
-    if (!store) [[unlikely]] {
-      THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-      return;
-    }
-#endif
+        data, size, SandboxBackingStoreDeleter, nullptr);
 
     buf = ArrayBuffer::New(isolate, std::move(store));
   }

src/node_i18n.cc

@@ -105,7 +105,12 @@ namespace {
 template <typename T>
 MaybeLocal<Object> ToBufferEndian(Environment* env, MaybeStackBuffer<T>* buf) {
   Local<Object> ret;
+#ifdef V8_ENABLE_SANDBOX
+  if (!Buffer::Copy(env, reinterpret_cast<char*>(buf->out()),
+                    buf->length() * sizeof(T)).ToLocal(&ret)) {
+#else
   if (!Buffer::New(env, buf).ToLocal(&ret)) {
+#endif
     return {};
   }
 
@@ -182,7 +187,12 @@ MaybeLocal<Object> TranscodeLatin1ToUcs2(Environment* env,
     return {};
   }
 
+#ifdef V8_ENABLE_SANDBOX
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()),
+                      destbuf.length() * sizeof(char16_t));
+#else
   return Buffer::New(env, &destbuf);
+#endif
 }
 
 MaybeLocal<Object> TranscodeFromUcs2(Environment* env,
@@ -227,7 +237,12 @@ MaybeLocal<Object> TranscodeUcs2FromUtf8(Environment* env,
     return {};
   }
 
+#ifdef V8_ENABLE_SANDBOX
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()),
+                      destbuf.length() * sizeof(char16_t));
+#else
   return Buffer::New(env, &destbuf);
+#endif
 }
 
 MaybeLocal<Object> TranscodeUtf8FromUcs2(Environment* env,
@@ -251,7 +266,12 @@ MaybeLocal<Object> TranscodeUtf8FromUcs2(Environment* env,
     return {};
   }
 
+#ifdef V8_ENABLE_SANDBOX
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()),
+                      destbuf.length() * sizeof(char));
+#else
   return Buffer::New(env, &destbuf);
+#endif
 }
 
 constexpr const char* EncodingName(const enum encoding encoding) {

src/node_sea.cc

@@ -11,6 +11,7 @@
 #include "node_snapshot_builder.h"
 #include "node_union_bytes.h"
 #include "node_v8_platform-inl.h"
+#include "node_v8_sandbox.h"
 #include "simdjson.h"
 #include "util-inl.h"
 
@@ -833,13 +834,19 @@ void GetAsset(const FunctionCallbackInfo<Value>& args) {
   if (it == sea_resource.assets.end()) {
     return;
   }
+#ifdef V8_ENABLE_SANDBOX
+  std::unique_ptr<v8::BackingStore> store = CopyCageBackingStore(
+      it->second.data(), it->second.size());
+  CHECK(store);
+#else
   // We cast away the constness here, the JS land should ensure that
   // the data is not mutated.
   std::unique_ptr<v8::BackingStore> store = ArrayBuffer::NewBackingStore(
       const_cast<char*>(it->second.data()),
       it->second.size(),
       [](void*, size_t, void*) {},
       nullptr);
+#endif
   Local<ArrayBuffer> ab = ArrayBuffer::New(args.GetIsolate(), std::move(store));
   args.GetReturnValue().Set(ab);
 }