src: support building with the V8 sandbox

With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be
allocated inside the sandbox, so memory that Node.js or a library
allocated itself cannot be wrapped and has to be copied in. Several
places already special-cased this, each with its own #ifdef, but a
build with the sandbox enabled still failed to compile
(`kMaxSafeBufferSizeForSandbox`), aborted in `crypto.randomUUID()`
(`SecureBuffer` was UNREACHABLE) and threw from every caller of the
malloc-owning `Buffer::New()` (v8.serialize, string transcoding, IPC,
the public `Buffer::New(isolate, data, length)`), and trace_events,
SEA assets and FFI still wrapped outside memory unconditionally.

Add `AdoptIntoBackingStore()`, which wraps the memory as before in
regular builds and copies it into an isolate-allocated backing store,
running the deleter right away, when the sandbox is enabled, and use it
at all of those sites so the #ifdef lives in one place. The trace
category flag cannot be copied since JS polls it, so under the sandbox
`getCategoryEnabledBuffer()` returns nothing and lib falls back to
`isTraceCategoryEnabled()`; FFI zero-copy views throw
ERR_OPERATION_FAILED there for the same reason.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

Author: codebytere

lib/ffi.js

@@ -256,11 +256,10 @@ function exportString(str, data, len, encoding = 'utf8') {
     throw new ERR_OUT_OF_RANGE('len', `>= ${requiredLength}`, len);
   }
 
-  const targetBuffer = toBuffer(data, len, false);
-  const dataLength = sourceBuffer.length;
-
-  sourceBuffer.copy(targetBuffer, 0, 0, dataLength);
-  targetBuffer.fill(0, dataLength, dataLength + terminatorSize);
+  const terminated = Buffer.allocUnsafe(requiredLength);
+  sourceBuffer.copy(terminated);
+  terminated.fill(0, sourceBuffer.length);
+  exportBytes(terminated, data, len);
 }
 
 function exportBuffer(source, data, len) {

lib/internal/http.js

@@ -10,7 +10,7 @@ const {
 
 const { setUnrefTimeout } = require('internal/timers');
 const {
-  getCategoryEnabledBuffer,
+  categoryEnabledChecker,
   trace,
   nodeTraceEventCategory,
   kAsyncBegin,
@@ -44,11 +44,7 @@ function getNextTraceEventId() {
   return ++traceEventId;
 }
 
-const httpEnabled = getCategoryEnabledBuffer('node.http');
-
-function isTraceHTTPEnabled() {
-  return httpEnabled[0] > 0;
-}
+const isTraceHTTPEnabled = categoryEnabledChecker('node.http');
 
 const traceEventCategory = nodeTraceEventCategory('node.http');
 

lib/internal/trace_events.js

@@ -1,6 +1,11 @@
 'use strict';
 
-const { getCategoryEnabledBuffer, trace, usePerfetto } = internalBinding('trace_events');
+const {
+  getCategoryEnabledBuffer,
+  isTraceCategoryEnabled,
+  trace,
+  usePerfetto,
+} = internalBinding('trace_events');
 const {
   CHAR_UPPERCASE_B,
   CHAR_LOWERCASE_B,
@@ -17,6 +22,16 @@ if (usePerfetto) {
   nodeTraceEventCategory = (category) => `node,${category}`;
 }
 
+// The returned function reads the category's enabled flag directly when the
+// binding can expose it to JS, and asks the tracing controller otherwise.
+function categoryEnabledChecker(category) {
+  const buffer = getCategoryEnabledBuffer(category);
+  if (buffer === undefined) {
+    return () => isTraceCategoryEnabled(category);
+  }
+  return () => buffer[0] > 0;
+}
+
 // The async events describe the execution of a single asynchronous operation, and are
 // used to measure the time spent in a single asynchronous operation.
 // Async events may overlap with each other. Different events do not have
@@ -44,7 +59,7 @@ const kTraceInstant = CHAR_LOWERCASE_N;
 
 module.exports = {
   usePerfetto,
-  getCategoryEnabledBuffer,
+  categoryEnabledChecker,
   trace,
   nodeTraceEventCategory,
   kAsyncBegin,

lib/internal/util/debuglog.js

@@ -16,7 +16,7 @@ const {
 } = primordials;
 const { inspect, format, formatWithOptions } = require('internal/util/inspect');
 const {
-  getCategoryEnabledBuffer,
+  categoryEnabledChecker,
   trace,
   nodeTraceEventCategory,
   kAsyncBegin,
@@ -388,14 +388,14 @@ function debugWithTimer(set, cb) {
   }
 
   const traceCategory = nodeTraceEventCategory(`node.${StringPrototypeToLowerCase(set)}`);
-  let traceCategoryBuffer;
+  let traceCategoryEnabled;
   let debugLogCategoryEnabled = false;
   let timerFlags = kNone;
 
   function ensureTimerFlagsAreUpdated() {
     timerFlags &= ~kSkipTrace;
 
-    if (traceCategoryBuffer[0] === 0) {
+    if (!traceCategoryEnabled()) {
       timerFlags |= kSkipTrace;
     }
   }
@@ -469,15 +469,15 @@ function debugWithTimer(set, cb) {
     }
     emitWarningIfNeeded(set);
     debugLogCategoryEnabled = testEnabled(set);
-    traceCategoryBuffer = getCategoryEnabledBuffer(traceCategory);
+    traceCategoryEnabled = categoryEnabledChecker(traceCategory);
 
     timerFlags = kNone;
 
     if (!debugLogCategoryEnabled) {
       timerFlags |= kSkipLog;
     }
 
-    if (traceCategoryBuffer[0] === 0) {
+    if (!traceCategoryEnabled()) {
       timerFlags |= kSkipTrace;
     }
 

src/crypto/crypto_dh.cc

@@ -22,8 +22,6 @@ using ncrypto::DHPointer;
 using ncrypto::EVPKeyCtxPointer;
 using ncrypto::EVPKeyPointer;
 using v8::ArrayBuffer;
-using v8::BackingStoreInitializationMode;
-using v8::BackingStoreOnFailureMode;
 using v8::ConstructorBehavior;
 using v8::Context;
 using v8::DontDelete;
@@ -60,21 +58,8 @@ MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
   struct Flag {
     bool secure;
   };
-#ifdef V8_ENABLE_SANDBOX
-  auto backing = ArrayBuffer::NewBackingStore(
+  auto backing = AdoptIntoBackingStore(
       env->isolate(),
-      data.size(),
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
-  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(),
       data.size(),
       [](void* data, size_t len, void* ptr) {
@@ -83,7 +68,10 @@ MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
       },
       new Flag{data.isSecure()});
   data.release();
-#endif  // V8_ENABLE_SANDBOX
+  if (!backing) {
+    THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
+    return MaybeLocal<Value>();
+  }
 
   auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
   return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Value>());

src/crypto/crypto_util.cc

@@ -32,8 +32,6 @@ using v8::Array;
 using v8::ArrayBuffer;
 using v8::ArrayBufferView;
 using v8::BackingStore;
-using v8::BackingStoreInitializationMode;
-using v8::BackingStoreOnFailureMode;
 using v8::BigInt;
 using v8::Context;
 using v8::EscapableHandleScope;
@@ -446,33 +444,18 @@ std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore(
   // It's ok for allocated_data_ to be nullptr but
   // 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(
+  std::unique_ptr<BackingStore> ptr = AdoptIntoBackingStore(
       env->isolate(),
+      allocated_data_,
       size(),
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
+      [](void* data, size_t length, void*) {
+        OPENSSL_clear_free(data, length);
+      },
+      nullptr);
   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(
-      allocated_data_,
-      size(),
-      [](void* data, size_t length, void* deleter_data) {
-        OPENSSL_clear_free(deleter_data, length);
-      }, allocated_data_);
-#endif  // V8_ENABLE_SANDBOX
-  CHECK(ptr);
   allocated_data_ = nullptr;
   data_ = nullptr;
   size_ = 0;
@@ -837,17 +820,6 @@ namespace {
 // 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();
 
@@ -857,7 +829,10 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
   }
   auto released = data.release();
 
-  std::shared_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
+  // Under V8_ENABLE_SANDBOX this ends up as a plain copy, which is fine:
+  // --secure-heap is unavailable there, so SecureAlloc() is OPENSSL_malloc().
+  std::shared_ptr<BackingStore> store = AdoptIntoBackingStore(
+      env->isolate(),
       released.data,
       released.len,
       [](void* data, size_t len, void* deleter_data) {
@@ -871,10 +846,12 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
             true);
       },
       nullptr);
+  if (!store) {
+    return THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
+  }
 
   Local<ArrayBuffer> buffer = ArrayBuffer::New(env->isolate(), store);
   args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len));
-#endif  // V8_ENABLE_SANDBOX
 }
 
 void SecureHeapUsed(const FunctionCallbackInfo<Value>& args) {

src/crypto/crypto_x509.cc

@@ -27,8 +27,6 @@ using ncrypto::X509View;
 using v8::Array;
 using v8::ArrayBuffer;
 using v8::ArrayBufferView;
-using v8::BackingStoreInitializationMode;
-using v8::BackingStoreOnFailureMode;
 using v8::Boolean;
 using v8::Context;
 using v8::Date;
@@ -140,29 +138,18 @@ MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer* bio) {
   BUF_MEM* mem = *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(
+  auto backing = AdoptIntoBackingStore(
       env->isolate(),
-      mem->length,
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
-  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,
       mem->length,
       [](void*, size_t, void* data) {
         BIOPointer free_me(static_cast<BIO*>(data));
       },
       bio->release());
-#endif  // V8_ENABLE_SANDBOX
+  if (!backing) {
+    THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
+    return MaybeLocal<Value>();
+  }
   auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
   Local<Value> ret;
   if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&ret)) return {};

src/ffi/data.cc

@@ -535,6 +535,17 @@ void ToString(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(out);
 }
 
+// Foreign memory lies outside the V8 sandbox and cannot back an ArrayBuffer.
+static bool ZeroCopyUnavailable(Environment* env) {
+#ifdef V8_ENABLE_SANDBOX
+  THROW_ERR_OPERATION_FAILED(
+      env, "Zero-copy views are not available when the V8 sandbox is enabled");
+  return true;
+#else
+  return false;
+#endif
+}
+
 void ToBuffer(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
@@ -580,9 +591,12 @@ void ToBuffer(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  bool copy = args.Length() < 3 || args[2]->IsUndefined() ||
+              args[2]->BooleanValue(isolate);
+  if (!copy && ZeroCopyUnavailable(env)) return;
+
   Local<Object> buf;
-  if (args.Length() < 3 || args[2]->IsUndefined() ||
-      args[2]->BooleanValue(isolate)) {
+  if (copy) {
     if (!Buffer::Copy(env, reinterpret_cast<char*>(ptr), len).ToLocal(&buf)) {
       return;
     }
@@ -642,10 +656,12 @@ void ToArrayBuffer(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  Local<ArrayBuffer> ab;
+  bool copy = args.Length() < 3 || args[2]->IsUndefined() ||
+              args[2]->BooleanValue(isolate);
+  if (!copy && ZeroCopyUnavailable(env)) return;
 
-  if (args.Length() < 3 || args[2]->IsUndefined() ||
-      args[2]->BooleanValue(isolate)) {
+  Local<ArrayBuffer> ab;
+  if (copy) {
     std::unique_ptr<BackingStore> store =
         ArrayBuffer::NewBackingStore(isolate, len);
     memcpy(store->Data(), reinterpret_cast<void*>(ptr), len);