|
| 1 | +From de3c7586220032e30e83529f24e0d63d2cfeed75 Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?=D0=A1=D0=BA=D0=BE=D0=B2=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20?= |
| 3 | + =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0=20=D0=90=D0=BD=D0=B4=D1=80?= |
| 4 | + =?UTF-8?q?=D0=B5=D0=B5=D0=B2=D0=B8=D1=87?= <chalkerx@gmail.com> |
| 5 | +Date: Fri, 7 Nov 2025 11:50:57 -0300 |
| 6 | +Subject: [PATCH] src,lib: refactor unsafe buffer creation to remove zero-fill |
| 7 | + toggle |
| 8 | + |
| 9 | +This removes the zero-fill toggle mechanism that allowed JavaScript |
| 10 | +to control ArrayBuffer initialization via shared memory. Instead, |
| 11 | +unsafe buffer creation now uses a dedicated C++ API. |
| 12 | + |
| 13 | +Refs: https://hackerone.com/reports/3405778 |
| 14 | +Co-Authored-By: Rafael Gonzaga <rafael.nunu@hotmail.com> |
| 15 | +Co-Authored-By: Joyee Cheung <joyeec9h3@gmail.com> |
| 16 | +Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> |
| 17 | +PR-URL: https://github.com/nodejs-private/node-private/pull/759 |
| 18 | +Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/799 |
| 19 | +CVE-ID: CVE-2025-55131 |
| 20 | +Signed-off-by: Azure Linux Security Servicing Account <azurelinux-security@microsoft.com> |
| 21 | +Upstream-reference: https://github.com/nodejs/node/commit/51f4de4b4a.patch |
| 22 | +--- |
| 23 | + deps/v8/include/v8-array-buffer.h | 7 +++ |
| 24 | + deps/v8/src/api/api.cc | 17 ++++++ |
| 25 | + lib/internal/buffer.js | 23 ++------ |
| 26 | + lib/internal/process/pre_execution.js | 2 - |
| 27 | + src/api/environment.cc | 3 +- |
| 28 | + src/node_buffer.cc | 84 ++++++++++++++++----------- |
| 29 | + 6 files changed, 82 insertions(+), 54 deletions(-) |
| 30 | + |
| 31 | +diff --git a/deps/v8/include/v8-array-buffer.h b/deps/v8/include/v8-array-buffer.h |
| 32 | +index 804fc42c..e03ed1a6 100644 |
| 33 | +--- a/deps/v8/include/v8-array-buffer.h |
| 34 | ++++ b/deps/v8/include/v8-array-buffer.h |
| 35 | +@@ -244,6 +244,13 @@ class V8_EXPORT ArrayBuffer : public Object { |
| 36 | + */ |
| 37 | + static std::unique_ptr<BackingStore> NewBackingStore(Isolate* isolate, |
| 38 | + size_t byte_length); |
| 39 | ++ /** |
| 40 | ++ * Returns a new standalone BackingStore with uninitialized memory and |
| 41 | ++ * return nullptr on failure. |
| 42 | ++ * This variant is for not breaking ABI on Node.js LTS. DO NOT USE. |
| 43 | ++ */ |
| 44 | ++ static std::unique_ptr<BackingStore> NewBackingStoreForNodeLTS( |
| 45 | ++ Isolate* isolate, size_t byte_length); |
| 46 | + /** |
| 47 | + * Returns a new standalone BackingStore that takes over the ownership of |
| 48 | + * the given buffer. The destructor of the BackingStore invokes the given |
| 49 | +diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc |
| 50 | +index a06394e6..da0c960f 100644 |
| 51 | +--- a/deps/v8/src/api/api.cc |
| 52 | ++++ b/deps/v8/src/api/api.cc |
| 53 | +@@ -8743,6 +8743,23 @@ std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore( |
| 54 | + static_cast<v8::BackingStore*>(backing_store.release())); |
| 55 | + } |
| 56 | + |
| 57 | ++std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStoreForNodeLTS( |
| 58 | ++ Isolate* v8_isolate, size_t byte_length) { |
| 59 | ++ i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate); |
| 60 | ++ API_RCS_SCOPE(i_isolate, ArrayBuffer, NewBackingStore); |
| 61 | ++ CHECK_LE(byte_length, i::JSArrayBuffer::kMaxByteLength); |
| 62 | ++ ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate); |
| 63 | ++ std::unique_ptr<i::BackingStoreBase> backing_store = |
| 64 | ++ i::BackingStore::Allocate(i_isolate, byte_length, |
| 65 | ++ i::SharedFlag::kNotShared, |
| 66 | ++ i::InitializedFlag::kUninitialized); |
| 67 | ++ if (!backing_store) { |
| 68 | ++ return nullptr; |
| 69 | ++ } |
| 70 | ++ return std::unique_ptr<v8::BackingStore>( |
| 71 | ++ static_cast<v8::BackingStore*>(backing_store.release())); |
| 72 | ++} |
| 73 | ++ |
| 74 | + std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore( |
| 75 | + void* data, size_t byte_length, v8::BackingStore::DeleterCallback deleter, |
| 76 | + void* deleter_data) { |
| 77 | +diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js |
| 78 | +index fbe9de24..23df382f 100644 |
| 79 | +--- a/lib/internal/buffer.js |
| 80 | ++++ b/lib/internal/buffer.js |
| 81 | +@@ -30,7 +30,7 @@ const { |
| 82 | + hexWrite, |
| 83 | + ucs2Write, |
| 84 | + utf8Write, |
| 85 | +- getZeroFillToggle, |
| 86 | ++ createUnsafeArrayBuffer, |
| 87 | + } = internalBinding('buffer'); |
| 88 | + |
| 89 | + const { |
| 90 | +@@ -1053,26 +1053,14 @@ function markAsUntransferable(obj) { |
| 91 | + obj[untransferable_object_private_symbol] = true; |
| 92 | + } |
| 93 | + |
| 94 | +-// A toggle used to access the zero fill setting of the array buffer allocator |
| 95 | +-// in C++. |
| 96 | +-// |zeroFill| can be undefined when running inside an isolate where we |
| 97 | +-// do not own the ArrayBuffer allocator. Zero fill is always on in that case. |
| 98 | +-let zeroFill = getZeroFillToggle(); |
| 99 | + function createUnsafeBuffer(size) { |
| 100 | +- zeroFill[0] = 0; |
| 101 | +- try { |
| 102 | ++ if (size <= 64) { |
| 103 | ++ // Allocated in heap, doesn't call backing store anyway |
| 104 | ++ // This is the same that the old impl did implicitly, but explicit now |
| 105 | + return new FastBuffer(size); |
| 106 | +- } finally { |
| 107 | +- zeroFill[0] = 1; |
| 108 | + } |
| 109 | +-} |
| 110 | + |
| 111 | +-// The connection between the JS land zero fill toggle and the |
| 112 | +-// C++ one in the NodeArrayBufferAllocator gets lost if the toggle |
| 113 | +-// is deserialized from the snapshot, because V8 owns the underlying |
| 114 | +-// memory of this toggle. This resets the connection. |
| 115 | +-function reconnectZeroFillToggle() { |
| 116 | +- zeroFill = getZeroFillToggle(); |
| 117 | ++ return new FastBuffer(createUnsafeArrayBuffer(size)); |
| 118 | + } |
| 119 | + |
| 120 | + module.exports = { |
| 121 | +@@ -1082,5 +1070,4 @@ module.exports = { |
| 122 | + createUnsafeBuffer, |
| 123 | + readUInt16BE, |
| 124 | + readUInt32BE, |
| 125 | +- reconnectZeroFillToggle, |
| 126 | + }; |
| 127 | +diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js |
| 128 | +index 16e29148..4a5dc638 100644 |
| 129 | +--- a/lib/internal/process/pre_execution.js |
| 130 | ++++ b/lib/internal/process/pre_execution.js |
| 131 | +@@ -27,7 +27,6 @@ const { |
| 132 | + refreshOptions, |
| 133 | + getEmbedderOptions, |
| 134 | + } = require('internal/options'); |
| 135 | +-const { reconnectZeroFillToggle } = require('internal/buffer'); |
| 136 | + const { |
| 137 | + exposeInterface, |
| 138 | + exposeLazyInterfaces, |
| 139 | +@@ -98,7 +97,6 @@ function prepareExecution(options) { |
| 140 | + const { expandArgv1, initializeModules, isMainThread } = options; |
| 141 | + |
| 142 | + refreshRuntimeOptions(); |
| 143 | +- reconnectZeroFillToggle(); |
| 144 | + |
| 145 | + // Patch the process object and get the resolved main entry point. |
| 146 | + const mainEntry = patchProcessObject(expandArgv1); |
| 147 | +diff --git a/src/api/environment.cc b/src/api/environment.cc |
| 148 | +index cdc2f7aa..776b6f10 100644 |
| 149 | +--- a/src/api/environment.cc |
| 150 | ++++ b/src/api/environment.cc |
| 151 | +@@ -107,8 +107,9 @@ void* NodeArrayBufferAllocator::Allocate(size_t size) { |
| 152 | + ret = allocator_->Allocate(size); |
| 153 | + else |
| 154 | + ret = allocator_->AllocateUninitialized(size); |
| 155 | +- if (LIKELY(ret != nullptr)) |
| 156 | ++ if (ret != nullptr) [[likely]] { |
| 157 | + total_mem_usage_.fetch_add(size, std::memory_order_relaxed); |
| 158 | ++ } |
| 159 | + return ret; |
| 160 | + } |
| 161 | + |
| 162 | +diff --git a/src/node_buffer.cc b/src/node_buffer.cc |
| 163 | +index e63318b6..6604e53e 100644 |
| 164 | +--- a/src/node_buffer.cc |
| 165 | ++++ b/src/node_buffer.cc |
| 166 | +@@ -74,7 +74,6 @@ using v8::Object; |
| 167 | + using v8::SharedArrayBuffer; |
| 168 | + using v8::String; |
| 169 | + using v8::Uint32; |
| 170 | +-using v8::Uint32Array; |
| 171 | + using v8::Uint8Array; |
| 172 | + using v8::Value; |
| 173 | + |
| 174 | +@@ -1170,35 +1169,6 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) { |
| 175 | + realm->set_buffer_prototype_object(proto); |
| 176 | + } |
| 177 | + |
| 178 | +-void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) { |
| 179 | +- Environment* env = Environment::GetCurrent(args); |
| 180 | +- NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator(); |
| 181 | +- Local<ArrayBuffer> ab; |
| 182 | +- // It can be a nullptr when running inside an isolate where we |
| 183 | +- // do not own the ArrayBuffer allocator. |
| 184 | +- if (allocator == nullptr) { |
| 185 | +- // Create a dummy Uint32Array - the JS land can only toggle the C++ land |
| 186 | +- // setting when the allocator uses our toggle. With this the toggle in JS |
| 187 | +- // land results in no-ops. |
| 188 | +- ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t)); |
| 189 | +- } else { |
| 190 | +- uint32_t* zero_fill_field = allocator->zero_fill_field(); |
| 191 | +- std::unique_ptr<BackingStore> backing = |
| 192 | +- ArrayBuffer::NewBackingStore(zero_fill_field, |
| 193 | +- sizeof(*zero_fill_field), |
| 194 | +- [](void*, size_t, void*) {}, |
| 195 | +- nullptr); |
| 196 | +- ab = ArrayBuffer::New(env->isolate(), std::move(backing)); |
| 197 | +- } |
| 198 | +- |
| 199 | +- ab->SetPrivate( |
| 200 | +- env->context(), |
| 201 | +- env->untransferable_object_private_symbol(), |
| 202 | +- True(env->isolate())).Check(); |
| 203 | +- |
| 204 | +- args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1)); |
| 205 | +-} |
| 206 | +- |
| 207 | + void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) { |
| 208 | + Environment* env = Environment::GetCurrent(args); |
| 209 | + if (args[0]->IsArrayBuffer()) { |
| 210 | +@@ -1378,6 +1348,54 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) { |
| 211 | + memcpy(dest, src, bytes_to_copy); |
| 212 | + } |
| 213 | + |
| 214 | ++// Converts a number parameter to size_t suitable for ArrayBuffer sizes |
| 215 | ++// Could be larger than uint32_t |
| 216 | ++// See v8::internal::TryNumberToSize and v8::internal::NumberToSize |
| 217 | ++inline size_t CheckNumberToSize(Local<Value> number) { |
| 218 | ++ CHECK(number->IsNumber()); |
| 219 | ++ double value = number.As<Number>()->Value(); |
| 220 | ++ // See v8::internal::TryNumberToSize on this (and on < comparison) |
| 221 | ++ double maxSize = static_cast<double>(std::numeric_limits<size_t>::max()); |
| 222 | ++ CHECK(value >= 0 && value < maxSize); |
| 223 | ++ size_t size = static_cast<size_t>(value); |
| 224 | ++#ifdef V8_ENABLE_SANDBOX |
| 225 | ++ CHECK_LE(size, kMaxSafeBufferSizeForSandbox); |
| 226 | ++#endif |
| 227 | ++ return size; |
| 228 | ++} |
| 229 | ++ |
| 230 | ++void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) { |
| 231 | ++ Environment* env = Environment::GetCurrent(args); |
| 232 | ++ if (args.Length() != 1) { |
| 233 | ++ env->ThrowRangeError("Invalid array buffer length"); |
| 234 | ++ return; |
| 235 | ++ } |
| 236 | ++ |
| 237 | ++ size_t size = CheckNumberToSize(args[0]); |
| 238 | ++ |
| 239 | ++ Isolate* isolate = env->isolate(); |
| 240 | ++ |
| 241 | ++ Local<ArrayBuffer> buf; |
| 242 | ++ |
| 243 | ++ NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator(); |
| 244 | ++ // 0-length, or zero-fill flag is set, or building snapshot |
| 245 | ++ if (size == 0 || per_process::cli_options->zero_fill_all_buffers || |
| 246 | ++ allocator == nullptr) { |
| 247 | ++ buf = ArrayBuffer::New(isolate, size); |
| 248 | ++ } else { |
| 249 | ++ std::unique_ptr<BackingStore> store = |
| 250 | ++ ArrayBuffer::NewBackingStoreForNodeLTS(isolate, size); |
| 251 | ++ if (!store) { |
| 252 | ++ // This slightly differs from the old behavior, |
| 253 | ++ // as in v8 that's a RangeError, and this is an Error with code |
| 254 | ++ return env->ThrowRangeError("Array buffer allocation failed"); |
| 255 | ++ } |
| 256 | ++ buf = ArrayBuffer::New(isolate, std::move(store)); |
| 257 | ++ } |
| 258 | ++ |
| 259 | ++ args.GetReturnValue().Set(buf); |
| 260 | ++} |
| 261 | ++ |
| 262 | + void Initialize(Local<Object> target, |
| 263 | + Local<Value> unused, |
| 264 | + Local<Context> context, |
| 265 | +@@ -1406,6 +1424,8 @@ void Initialize(Local<Object> target, |
| 266 | + |
| 267 | + SetMethod(context, target, "detachArrayBuffer", DetachArrayBuffer); |
| 268 | + SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer); |
| 269 | ++ SetMethodNoSideEffect( |
| 270 | ++ context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer); |
| 271 | + |
| 272 | + SetMethod(context, target, "swap16", Swap16); |
| 273 | + SetMethod(context, target, "swap32", Swap32); |
| 274 | +@@ -1442,8 +1462,6 @@ void Initialize(Local<Object> target, |
| 275 | + SetMethod(context, target, "hexWrite", StringWrite<HEX>); |
| 276 | + SetMethod(context, target, "ucs2Write", StringWrite<UCS2>); |
| 277 | + SetMethod(context, target, "utf8Write", StringWrite<UTF8>); |
| 278 | +- |
| 279 | +- SetMethod(context, target, "getZeroFillToggle", GetZeroFillToggle); |
| 280 | + } |
| 281 | + |
| 282 | + } // anonymous namespace |
| 283 | +@@ -1485,10 +1503,10 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { |
| 284 | + registry->Register(StringWrite<HEX>); |
| 285 | + registry->Register(StringWrite<UCS2>); |
| 286 | + registry->Register(StringWrite<UTF8>); |
| 287 | +- registry->Register(GetZeroFillToggle); |
| 288 | + |
| 289 | + registry->Register(DetachArrayBuffer); |
| 290 | + registry->Register(CopyArrayBuffer); |
| 291 | ++ registry->Register(CreateUnsafeArrayBuffer); |
| 292 | + |
| 293 | + registry->Register(Atob); |
| 294 | + registry->Register(Btoa); |
| 295 | +-- |
| 296 | +2.45.4 |
| 297 | + |
0 commit comments