electron/patches/node/support_v8_sandboxed_pointers.patch

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Rose <japthorp@slack-corp.com>
Date: Tue, 21 Jun 2022 10:04:21 -0700
Subject: support V8 sandboxed pointers

This refactors several allocators to allocate within the V8 memory cage,
allowing them to be compatible with the V8_SANDBOXED_POINTERS feature.

diff --git a/src/api/environment.cc b/src/api/environment.cc
index d10f861c96931d06fb50dcdb66f2e79b0dee55a7..a869bc0a145009b57db3f37208e405d9356cc20f 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -109,6 +109,14 @@ MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
   return result;
 }
 
+NodeArrayBufferAllocator::NodeArrayBufferAllocator() {
+  zero_fill_field_ = static_cast<uint32_t*>(allocator_->Allocate(sizeof(*zero_fill_field_)));
+}
+
+NodeArrayBufferAllocator::~NodeArrayBufferAllocator() {
+  allocator_->Free(zero_fill_field_, sizeof(*zero_fill_field_));
+}
+
 void* NodeArrayBufferAllocator::Allocate(size_t size) {
   void* ret;
   if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
@@ -324,6 +332,12 @@ Isolate* NewIsolate(Isolate::CreateParams* params,
     // but also otherwise just doesn't work, and the only real alternative
     // is disabling shared-readonly-heap mode altogether.
     static Isolate::CreateParams first_params = *params;
+    // Clear allocator pointers to prevent use-after-free during static
+    // destruction. The static first_params can outlive V8's internal
+    // allocator systems, causing crashes when its destructor tries to
+    // free resources after V8 has shut down.
+    first_params.array_buffer_allocator = nullptr;
+    first_params.array_buffer_allocator_shared.reset();
     params->snapshot_blob = first_params.snapshot_blob;
     params->external_references = first_params.external_references;
   }
diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc
index 46a7d1396dc1a175ae99f4e403721f1730fdd320..bbb0abb3b9563074d350578e0f5a8fa211046b17 100644
--- a/src/crypto/crypto_dh.cc
+++ b/src/crypto/crypto_dh.cc
@@ -61,17 +61,22 @@ 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);
-  if (!backing) {
-    THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-    return MaybeLocal<Value>();
-  }
+  std::unique_ptr<v8::BackingStore> backing;
   if (data.size() > 0) {
-    memcpy(backing->Data(), data.get(), data.size());
+    std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+    void* v8_data = allocator->Allocate(data.size());
+    CHECK(v8_data);
+    memcpy(v8_data, data.get(), data.size());
+    backing = ArrayBuffer::NewBackingStore(
+        v8_data,
+        data.size(),
+        [](void* data, size_t length, void*) {
+          std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+          allocator->Free(data, length);
+        }, nullptr);
+  } else {
+    NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
+    backing = v8::ArrayBuffer::NewBackingStore(env->isolate(), data.size());
   }
 #else
   auto backing = ArrayBuffer::NewBackingStore(
diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index 12b0d804c6f1d4998b85160b0aac8eb7a3b5576b..27bd93769233dc65a064710db4095d9cdc3a8b1a 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -346,24 +346,30 @@ 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.
+#if defined(V8_ENABLE_SANDBOX)
+  // When V8 sandboxed pointers are enabled, we have to copy into the memory
+  // cage. We still want to ensure we erase the data on free though, so
+  // provide a custom deleter that calls OPENSSL_cleanse.
+  if (!size())
+    return ArrayBuffer::NewBackingStore(env->isolate(), 0);
+  std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+  void* v8_data = allocator->Allocate(size());
+  CHECK(v8_data);
+  memcpy(v8_data, allocated_data_, size());
+  OPENSSL_clear_free(allocated_data_, size());
   std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
-      env->isolate(),
+      v8_data,
       size(),
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
-  if (!ptr) {
-    THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-    return nullptr;
-  }
-  memcpy(ptr->Data(), allocated_data_, size());
-  OPENSSL_clear_free(allocated_data_, size_);
+      [](void* data, size_t length, void*) {
+        OPENSSL_cleanse(data, length);
+        std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+        allocator->Free(data, length);
+      }, nullptr);
+  CHECK(ptr);
+  allocated_data_ = nullptr;
+  data_ = nullptr;
+  size_ = 0;
+  return ptr;
 #else
   std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
       allocated_data_,
@@ -662,23 +668,16 @@ 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);
+  CHECK(args[0]->IsUint32());
 #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();
+   uint32_t len = args[0].As<Uint32>()->Value();
+   Local<ArrayBuffer> buffer = ArrayBuffer::New(args.GetIsolate(), len);
+   args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len));
 #else
-  CHECK(args[0]->IsUint32());
+  Environment* env = Environment::GetCurrent(args);
   uint32_t len = args[0].As<Uint32>()->Value();
 
   auto data = DataPointer::SecureAlloc(len);
-  CHECK(data.isSecure());
   if (!data) {
     return THROW_ERR_OPERATION_FAILED(env, "Allocation failed");
   }
diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc
index b30297eac08ad9587642b723f91d7e3b954294d4..4c5427596d1c90d3a413cdd9ff4f1151e657073d 100644
--- a/src/crypto/crypto_x509.cc
+++ b/src/crypto/crypto_x509.cc
@@ -135,19 +135,17 @@ MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer* bio) {
     return {};
   BUF_MEM* mem = *bio;
 #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(),
+  std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+  void* v8_data = allocator->Allocate(mem->length);
+  CHECK(v8_data);
+  memcpy(v8_data, mem->data, mem->length);
+  std::unique_ptr<v8::BackingStore> backing = ArrayBuffer::NewBackingStore(
+      v8_data,
       mem->length,
-      BackingStoreInitializationMode::kUninitialized,
-      BackingStoreOnFailureMode::kReturnNull);
-  if (!backing) {
-    THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-    return MaybeLocal<Value>();
-  }
-  memcpy(backing->Data(), mem->data, mem->length);
+      [](void* data, size_t length, void*) {
+        std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+        allocator->Free(data, length);
+      }, nullptr);
 #else
   auto backing = ArrayBuffer::NewBackingStore(
       mem->data,
diff --git a/src/env-inl.h b/src/env-inl.h
index 97c43afb487b58c0c77bd59b4a6b6d7a13690053..5dfbd564d5bbd22ebf3b529a07b73e85cbe51986 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -44,6 +44,16 @@
 
 namespace node {
 
+NoArrayBufferZeroFillScope::NoArrayBufferZeroFillScope(
+    IsolateData* isolate_data)
+    : node_allocator_(isolate_data->node_allocator()) {
+  if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 0;
+}
+
+NoArrayBufferZeroFillScope::~NoArrayBufferZeroFillScope() {
+  if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 1;
+}
+
 inline v8::Isolate* IsolateData::isolate() const {
   return isolate_;
 }
diff --git a/src/env.h b/src/env.h
index d34aec43630b3cf53004d8180446d7136b59ceac..b30314d7773742e2332ff47f84bc326151563690 100644
--- a/src/env.h
+++ b/src/env.h
@@ -111,6 +111,19 @@ class ModuleWrap;
 class Environment;
 class Realm;
 
+// Disables zero-filling for ArrayBuffer allocations in this scope. This is
+// similar to how we implement Buffer.allocUnsafe() in JS land.
+class NoArrayBufferZeroFillScope {
+ public:
+  inline explicit NoArrayBufferZeroFillScope(IsolateData* isolate_data);
+  inline ~NoArrayBufferZeroFillScope();
+
+ private:
+  NodeArrayBufferAllocator* node_allocator_;
+
+  friend class Environment;
+};
+
 struct IsolateDataSerializeInfo {
   std::vector<SnapshotIndex> primitive_values;
   std::vector<PropInfo> template_values;
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
index 3c4f419aa29470b3280174b58680b9421b0340b5..3b24ad2a2316f89d98b067e2c13988f87a9a00d2 100644
--- a/src/node_i18n.cc
+++ b/src/node_i18n.cc
@@ -105,7 +105,7 @@ namespace {
 template <typename T>
 MaybeLocal<Object> ToBufferEndian(Environment* env, MaybeStackBuffer<T>* buf) {
   Local<Object> ret;
-  if (!Buffer::New(env, buf).ToLocal(&ret)) {
+  if (!Buffer::Copy(env, reinterpret_cast<char*>(buf->out()), buf->length() * sizeof(T)).ToLocal(&ret)) {
     return {};
   }
 
@@ -182,7 +182,7 @@ MaybeLocal<Object> TranscodeLatin1ToUcs2(Environment* env,
     return {};
   }
 
-  return Buffer::New(env, &destbuf);
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()), destbuf.length() * sizeof(UChar));
 }
 
 MaybeLocal<Object> TranscodeFromUcs2(Environment* env,
@@ -227,7 +227,7 @@ MaybeLocal<Object> TranscodeUcs2FromUtf8(Environment* env,
     return {};
   }
 
-  return Buffer::New(env, &destbuf);
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()), destbuf.length() * sizeof(UChar));
 }
 
 MaybeLocal<Object> TranscodeUtf8FromUcs2(Environment* env,
@@ -251,7 +251,7 @@ MaybeLocal<Object> TranscodeUtf8FromUcs2(Environment* env,
     return {};
   }
 
-  return Buffer::New(env, &destbuf);
+  return Buffer::Copy(env, reinterpret_cast<char*>(destbuf.out()), destbuf.length() * sizeof(char));
 }
 
 constexpr const char* EncodingName(const enum encoding encoding) {
diff --git a/src/node_internals.h b/src/node_internals.h
index 12ea72b61b0a5e194207bb369dfed4b8667107cb..64442215714a98f648971e517ddd9c77e38fe3f2 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -121,7 +121,9 @@ v8::MaybeLocal<v8::Object> InitializePrivateSymbols(
 
 class NodeArrayBufferAllocator : public ArrayBufferAllocator {
  public:
-  inline uint32_t* zero_fill_field() { return &zero_fill_field_; }
+  NodeArrayBufferAllocator();
+  ~NodeArrayBufferAllocator() override;
+  inline uint32_t* zero_fill_field() { return zero_fill_field_; }
 
   void* Allocate(size_t size) override;  // Defined in src/node.cc
   void* AllocateUninitialized(size_t size) override;
@@ -139,7 +141,7 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator {
   }
 
  private:
-  uint32_t zero_fill_field_ = 1;  // Boolean but exposed as uint32 to JS land.
+  uint32_t* zero_fill_field_ = nullptr;  // Boolean but exposed as uint32 to JS land.
   std::atomic<size_t> total_mem_usage_ {0};
 
   // Delegate to V8's allocator for compatibility with the V8 memory cage.
diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 00fcd4b6afccce47ff21c4447d9cd60f25c11835..5f96ee2051e5339456185efddb149c4d43093f31 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -29,6 +29,26 @@ using v8::ValueSerializer;
 
 namespace serdes {
 
+v8::ArrayBuffer::Allocator* GetAllocator() {
+  static v8::ArrayBuffer::Allocator* allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
+  return allocator;
+}
+
+void* Reallocate(void* data, size_t old_length,
+                 size_t new_length) {
+  if (old_length == new_length) return data;
+  uint8_t* new_data =
+      reinterpret_cast<uint8_t*>(GetAllocator()->AllocateUninitialized(new_length));
+  if (new_data == nullptr) return nullptr;
+  size_t bytes_to_copy = std::min(old_length, new_length);
+  memcpy(new_data, data, bytes_to_copy);
+  if (new_length > bytes_to_copy) {
+    memset(new_data + bytes_to_copy, 0, new_length - bytes_to_copy);
+  }
+  GetAllocator()->Free(data, old_length);
+  return new_data;
+}
+
 class SerializerContext : public BaseObject,
                           public ValueSerializer::Delegate {
  public:
@@ -37,10 +57,15 @@ class SerializerContext : public BaseObject,
 
   ~SerializerContext() override = default;
 
+  // v8::ValueSerializer::Delegate
   void ThrowDataCloneError(Local<String> message) override;
   Maybe<bool> WriteHostObject(Isolate* isolate, Local<Object> object) override;
   Maybe<uint32_t> GetSharedArrayBufferId(
       Isolate* isolate, Local<SharedArrayBuffer> shared_array_buffer) override;
+  void* ReallocateBufferMemory(void* old_buffer,
+                               size_t old_length,
+                               size_t* new_length) override;
+  void FreeBufferMemory(void* buffer) override;
 
   static void SetTreatArrayBufferViewsAsHostObjects(
       const FunctionCallbackInfo<Value>& args);
@@ -61,6 +86,7 @@ class SerializerContext : public BaseObject,
 
  private:
   ValueSerializer serializer_;
+  size_t last_length_ = 0;
 };
 
 class DeserializerContext : public BaseObject,
@@ -145,6 +171,24 @@ Maybe<uint32_t> SerializerContext::GetSharedArrayBufferId(
   return id->Uint32Value(env()->context());
 }
 
+void* SerializerContext::ReallocateBufferMemory(void* old_buffer,
+                                                size_t requested_size,
+                                                size_t* new_length) {
+  *new_length = std::max(static_cast<size_t>(4096), requested_size);
+  if (old_buffer) {
+    void* ret = Reallocate(old_buffer, last_length_, *new_length);
+    last_length_ = *new_length;
+    return ret;
+  } else {
+    last_length_ = *new_length;
+    return GetAllocator()->Allocate(*new_length);
+  }
+}
+
+void SerializerContext::FreeBufferMemory(void* buffer) {
+  GetAllocator()->Free(buffer, last_length_);
+}
+
 Maybe<bool> SerializerContext::WriteHostObject(Isolate* isolate,
                                                Local<Object> input) {
   Local<Value> args[1] = { input };
@@ -211,10 +255,17 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
   // Note: Both ValueSerializer and this Buffer::New() variant use malloc()
   // as the underlying allocator.
   std::pair<uint8_t*, size_t> ret = ctx->serializer_.Release();
-  Local<Object> buf;
-  if (Buffer::New(ctx->env(), reinterpret_cast<char*>(ret.first), ret.second)
-          .ToLocal(&buf)) {
-    args.GetReturnValue().Set(buf);
+  std::unique_ptr<v8::BackingStore> bs =
+      v8::ArrayBuffer::NewBackingStore(reinterpret_cast<char*>(ret.first), ret.second,
+        [](void* data, size_t length, void* deleter_data) {
+          if (data) GetAllocator()->Free(reinterpret_cast<char*>(data), length);
+        }, nullptr);
+  Local<ArrayBuffer> ab = v8::ArrayBuffer::New(ctx->env()->isolate(), std::move(bs));
+
+  auto buf = Buffer::New(ctx->env(), ab, 0, ret.second);
+
+  if (!buf.IsEmpty()) {
+    args.GetReturnValue().Set(buf.ToLocalChecked());
   }
 }
 
diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc
index ef659f1c39f7ee958879bf395377bc99911fc346..225b1465b7c97d972a38968faf6d685017a80bf0 100644
--- a/src/node_trace_events.cc
+++ b/src/node_trace_events.cc
@@ -129,12 +129,28 @@ static void GetCategoryEnabledBuffer(const FunctionCallbackInfo<Value>& args) {
   const uint8_t* enabled_pointer =
       TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(category_name.out());
   uint8_t* enabled_pointer_cast = const_cast<uint8_t*>(enabled_pointer);
+  uint8_t size = sizeof(*enabled_pointer_cast);
 
+#if defined(V8_ENABLE_SANDBOX)
+  std::unique_ptr<ArrayBuffer::Allocator> allocator(ArrayBuffer::Allocator::NewDefaultAllocator());
+  void* v8_data = allocator->Allocate(size);
+  CHECK(v8_data);
+  memcpy(v8_data, enabled_pointer_cast, size);
+  std::unique_ptr<BackingStore> bs = ArrayBuffer::NewBackingStore(
+      v8_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> bs = ArrayBuffer::NewBackingStore(
       enabled_pointer_cast,
-      sizeof(*enabled_pointer_cast),
+      size,
       [](void*, size_t, void*) {},
       nullptr);
+#endif
+
   auto ab = ArrayBuffer::New(isolate, std::move(bs));
   v8::Local<Uint8Array> u8 = v8::Uint8Array::New(ab, 0, 1);
 
diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js
index 07e38748b07ddd32a6e3caa3c34183387e1cae18..f09bf0940dad2068f0aa5dce783dd422773d4bbb 100644
--- a/test/parallel/test-process-env-allowed-flags-are-documented.js
+++ b/test/parallel/test-process-env-allowed-flags-are-documented.js
@@ -49,6 +49,8 @@ if (!hasOpenSSL3) {
   documented.delete('--openssl-shared-config');
 }
 
+const isV8Sandboxed = process.config.variables.v8_enable_sandbox;
+
 // Filter out options that are conditionally present.
 const conditionalOpts = [
   {
@@ -74,6 +76,9 @@ const conditionalOpts = [
   }, {
     include: process.features.inspector,
     filter: (opt) => opt.startsWith('--inspect') || opt === '--debug-port'
+  }, {
+    include: !isV8Sandboxed,
+    filter: (opt) => ['--secure-heap', '--secure-heap-min'].includes(opt)
   },
 ];
 documented.forEach((opt) => {