src,lib: refactor unsafe buffer creation to remove zero-fill toggle

ChALkeR · RafaelGSS · joyeecheung · RafaelGSS · commit 51f4de4b4a52 · 2026-01-09T11:44:02.000-03:00
This removes the zero-fill toggle mechanism that allowed JavaScript to control ArrayBuffer initialization via shared memory. Instead, unsafe buffer creation now uses a dedicated C++ API. Refs: https://hackerone.com/reports/3405778 Co-Authored-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Co-Authored-By: Joyee Cheung <joyeec9h3@gmail.com> Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#759 Backport-PR-URL: nodejs-private/node-private#799 CVE-ID: CVE-2025-55131
diff --git a/deps/v8/include/v8-array-buffer.h b/deps/v8/include/v8-array-buffer.h
@@ -244,6 +244,13 @@ class V8_EXPORT ArrayBuffer : public Object {
    */
   static std::unique_ptr<BackingStore> NewBackingStore(Isolate* isolate,
                                                        size_t byte_length);
+  /**
+   * Returns a new standalone BackingStore with uninitialized memory and
+   * return nullptr on failure.
+   * This variant is for not breaking ABI on Node.js LTS. DO NOT USE.
+   */
+  static std::unique_ptr<BackingStore> NewBackingStoreForNodeLTS(
+      Isolate* isolate, size_t byte_length);
   /**
    * Returns a new standalone BackingStore that takes over the ownership of
    * the given buffer. The destructor of the BackingStore invokes the given
diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc
@@ -8743,6 +8743,23 @@ std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
       static_cast<v8::BackingStore*>(backing_store.release()));
 }
 
+std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStoreForNodeLTS(
+    Isolate* v8_isolate, size_t byte_length) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  API_RCS_SCOPE(i_isolate, ArrayBuffer, NewBackingStore);
+  CHECK_LE(byte_length, i::JSArrayBuffer::kMaxByteLength);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
+  std::unique_ptr<i::BackingStoreBase> backing_store =
+      i::BackingStore::Allocate(i_isolate, byte_length,
+                                i::SharedFlag::kNotShared,
+                                i::InitializedFlag::kUninitialized);
+  if (!backing_store) {
+    return nullptr;
+  }
+  return std::unique_ptr<v8::BackingStore>(
+      static_cast<v8::BackingStore*>(backing_store.release()));
+}
+
 std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
     void* data, size_t byte_length, v8::BackingStore::DeleterCallback deleter,
     void* deleter_data) {
diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -30,7 +30,7 @@ const {
   hexWrite,
   ucs2Write,
   utf8Write,
-  getZeroFillToggle,
+  createUnsafeArrayBuffer,
 } = internalBinding('buffer');
 
 const {
@@ -1053,26 +1053,14 @@ function markAsUntransferable(obj) {
   obj[untransferable_object_private_symbol] = true;
 }
 
-// A toggle used to access the zero fill setting of the array buffer allocator
-// in C++.
-// |zeroFill| can be undefined when running inside an isolate where we
-// do not own the ArrayBuffer allocator.  Zero fill is always on in that case.
-let zeroFill = getZeroFillToggle();
 function createUnsafeBuffer(size) {
-  zeroFill[0] = 0;
-  try {
+  if (size <= 64) {
+    // Allocated in heap, doesn't call backing store anyway
+    // This is the same that the old impl did implicitly, but explicit now
     return new FastBuffer(size);
-  } finally {
-    zeroFill[0] = 1;
   }
-}
 
-// The connection between the JS land zero fill toggle and the
-// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
-// is deserialized from the snapshot, because V8 owns the underlying
-// memory of this toggle. This resets the connection.
-function reconnectZeroFillToggle() {
-  zeroFill = getZeroFillToggle();
+  return new FastBuffer(createUnsafeArrayBuffer(size));
 }
 
 module.exports = {
@@ -1082,5 +1070,4 @@ module.exports = {
   createUnsafeBuffer,
   readUInt16BE,
   readUInt32BE,
-  reconnectZeroFillToggle,
 };
diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -24,7 +24,6 @@ const {
   refreshOptions,
   getEmbedderOptions,
 } = require('internal/options');
-const { reconnectZeroFillToggle } = require('internal/buffer');
 const {
   exposeInterface,
   exposeLazyInterfaces,
@@ -98,7 +97,6 @@ function prepareExecution(options) {
   const { expandArgv1, initializeModules, isMainThread } = options;
 
   refreshRuntimeOptions();
-  reconnectZeroFillToggle();
 
   // Patch the process object and get the resolved main entry point.
   const mainEntry = patchProcessObject(expandArgv1);
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -107,8 +107,9 @@ void* NodeArrayBufferAllocator::Allocate(size_t size) {
     ret = allocator_->Allocate(size);
   else
     ret = allocator_->AllocateUninitialized(size);
-  if (LIKELY(ret != nullptr))
+  if (ret != nullptr) [[likely]] {
     total_mem_usage_.fetch_add(size, std::memory_order_relaxed);
+  }
   return ret;
 }
 
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -75,7 +75,6 @@ using v8::Object;
 using v8::SharedArrayBuffer;
 using v8::String;
 using v8::Uint32;
-using v8::Uint32Array;
 using v8::Uint8Array;
 using v8::Value;
 
@@ -1177,35 +1176,6 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
   realm->set_buffer_prototype_object(proto);
 }
 
-void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
-  Local<ArrayBuffer> ab;
-  // It can be a nullptr when running inside an isolate where we
-  // do not own the ArrayBuffer allocator.
-  if (allocator == nullptr) {
-    // Create a dummy Uint32Array - the JS land can only toggle the C++ land
-    // setting when the allocator uses our toggle. With this the toggle in JS
-    // land results in no-ops.
-    ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
-  } else {
-    uint32_t* zero_fill_field = allocator->zero_fill_field();
-    std::unique_ptr<BackingStore> backing =
-        ArrayBuffer::NewBackingStore(zero_fill_field,
-                                     sizeof(*zero_fill_field),
-                                     [](void*, size_t, void*) {},
-                                     nullptr);
-    ab = ArrayBuffer::New(env->isolate(), std::move(backing));
-  }
-
-  ab->SetPrivate(
-      env->context(),
-      env->untransferable_object_private_symbol(),
-      True(env->isolate())).Check();
-
-  args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
-}
-
 void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   if (args[0]->IsArrayBuffer()) {
@@ -1397,6 +1367,54 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   memcpy(dest, src, bytes_to_copy);
 }
 
+// Converts a number parameter to size_t suitable for ArrayBuffer sizes
+// Could be larger than uint32_t
+// See v8::internal::TryNumberToSize and v8::internal::NumberToSize
+inline size_t CheckNumberToSize(Local<Value> number) {
+  CHECK(number->IsNumber());
+  double value = number.As<Number>()->Value();
+  // See v8::internal::TryNumberToSize on this (and on < comparison)
+  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, kMaxSafeBufferSizeForSandbox);
+#endif
+  return size;
+}
+
+void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  if (args.Length() != 1) {
+    env->ThrowRangeError("Invalid array buffer length");
+    return;
+  }
+
+  size_t size = CheckNumberToSize(args[0]);
+
+  Isolate* isolate = env->isolate();
+
+  Local<ArrayBuffer> buf;
+
+  NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
+  // 0-length, or zero-fill flag is set, or building snapshot
+  if (size == 0 || per_process::cli_options->zero_fill_all_buffers ||
+      allocator == nullptr) {
+    buf = ArrayBuffer::New(isolate, size);
+  } else {
+    std::unique_ptr<BackingStore> store =
+        ArrayBuffer::NewBackingStoreForNodeLTS(isolate, size);
+    if (!store) {
+      // This slightly differs from the old behavior,
+      // as in v8 that's a RangeError, and this is an Error with code
+      return env->ThrowRangeError("Array buffer allocation failed");
+    }
+    buf = ArrayBuffer::New(isolate, std::move(store));
+  }
+
+  args.GetReturnValue().Set(buf);
+}
+
 void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context,
@@ -1428,6 +1446,8 @@ void Initialize(Local<Object> target,
 
   SetMethod(context, target, "detachArrayBuffer", DetachArrayBuffer);
   SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer);
+  SetMethodNoSideEffect(
+      context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer);
 
   SetMethod(context, target, "swap16", Swap16);
   SetMethod(context, target, "swap32", Swap32);
@@ -1464,8 +1484,6 @@ void Initialize(Local<Object> target,
   SetMethod(context, target, "hexWrite", StringWrite<HEX>);
   SetMethod(context, target, "ucs2Write", StringWrite<UCS2>);
   SetMethod(context, target, "utf8Write", StringWrite<UTF8>);
-
-  SetMethod(context, target, "getZeroFillToggle", GetZeroFillToggle);
 }
 
 }  // anonymous namespace
@@ -1508,10 +1526,10 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(StringWrite<HEX>);
   registry->Register(StringWrite<UCS2>);
   registry->Register(StringWrite<UTF8>);
-  registry->Register(GetZeroFillToggle);
 
   registry->Register(DetachArrayBuffer);
   registry->Register(CopyArrayBuffer);
+  registry->Register(CreateUnsafeArrayBuffer);
 
   registry->Register(Atob);
   registry->Register(Btoa);
