From 8cfc8b0e7b55b21a1e92d25f5b42a5c05bff9e87 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 17 Aug 2022 20:36:33 +0200 Subject: [PATCH] lib: refactor to avoid prototype pollution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/43474 Reviewed-By: Matteo Collina Reviewed-By: Juan José Arboleda Reviewed-By: Rich Trott --- lib/internal/event_target.js | 8 ++++++++ lib/internal/util.js | 14 ++++++++++---- lib/internal/worker/io.js | 10 +++++++++- lib/util.js | 7 +++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 5388b5c8139a10..2be1b9cf0b9f39 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -11,6 +11,8 @@ const { ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, ObjectGetOwnPropertyDescriptors, + ObjectSetPrototypeOf, + ObjectValues, ReflectApply, SafeArrayIterator, SafeFinalizationRegistry, @@ -1062,6 +1064,12 @@ const EventEmitterMixin = (Superclass) => { } const protoProps = ObjectGetOwnPropertyDescriptors(EventEmitter.prototype); delete protoProps.constructor; + const propertiesValues = ObjectValues(protoProps); + for (let i = 0; i < propertiesValues.length; i++) { + // We want to use null-prototype objects to not rely on globally mutable + // %Object.prototype%. + ObjectSetPrototypeOf(propertiesValues[i], null); + } ObjectDefineProperties(MixedEventEmitter.prototype, protoProps); return MixedEventEmitter; }; diff --git a/lib/internal/util.js b/lib/internal/util.js index c077f4be2334a5..15fc5ad46b7629 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -16,6 +16,7 @@ const { ObjectFreeze, ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, + ObjectValues, Promise, ReflectApply, ReflectConstruct, @@ -369,10 +370,15 @@ function promisify(original) { __proto__: null, value: fn, enumerable: false, writable: false, configurable: true }); - return ObjectDefineProperties( - fn, - ObjectGetOwnPropertyDescriptors(original) - ); + + const descriptors = ObjectGetOwnPropertyDescriptors(original); + const propertiesValues = ObjectValues(descriptors); + for (let i = 0; i < propertiesValues.length; i++) { + // We want to use null-prototype objects to not rely on globally mutable + // %Object.prototype%. + ObjectSetPrototypeOf(propertiesValues[i], null); + } + return ObjectDefineProperties(fn, descriptors); } promisify.custom = kCustomPromisifiedSymbol; diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js index 9e2bbbdb33fd3e..61f9a5363716a8 100644 --- a/lib/internal/worker/io.js +++ b/lib/internal/worker/io.js @@ -13,6 +13,7 @@ const { ObjectGetOwnPropertyDescriptors, ObjectGetPrototypeOf, ObjectSetPrototypeOf, + ObjectValues, ReflectApply, Symbol, SymbolFor, @@ -95,10 +96,17 @@ const messageTypes = { // it inherit from NodeEventTarget, even though it is a C++ class, and b) we do // not provide methods that are not present in the Browser and not documented // on our side (e.g. stopMessagePort). +const messagePortPrototypePropertyDescriptors = ObjectGetOwnPropertyDescriptors(MessagePort.prototype); +const propertiesValues = ObjectValues(messagePortPrototypePropertyDescriptors); +for (let i = 0; i < propertiesValues.length; i++) { + // We want to use null-prototype objects to not rely on globally mutable + // %Object.prototype%. + ObjectSetPrototypeOf(propertiesValues[i], null); +} // Save a copy of the original set of methods as a shallow clone. const MessagePortPrototype = ObjectCreate( ObjectGetPrototypeOf(MessagePort.prototype), - ObjectGetOwnPropertyDescriptors(MessagePort.prototype)); + messagePortPrototypePropertyDescriptors); // Set up the new inheritance chain. ObjectSetPrototypeOf(MessagePort, NodeEventTarget); ObjectSetPrototypeOf(MessagePort.prototype, NodeEventTarget.prototype); diff --git a/lib/util.js b/lib/util.js index 78e6b807ee5ef6..bca74b6d5c649c 100644 --- a/lib/util.js +++ b/lib/util.js @@ -40,6 +40,7 @@ const { ObjectKeys, ObjectPrototypeToString, ObjectSetPrototypeOf, + ObjectValues, ReflectApply, StringPrototypePadStart, } = primordials; @@ -317,6 +318,12 @@ function callbackify(original) { if (typeof descriptors.name.value === 'string') { descriptors.name.value += 'Callbackified'; } + const propertiesValues = ObjectValues(descriptors); + for (let i = 0; i < propertiesValues.length; i++) { + // We want to use null-prototype objects to not rely on globally mutable + // %Object.prototype%. + ObjectSetPrototypeOf(propertiesValues[i], null); + } ObjectDefineProperties(callbackified, descriptors); return callbackified; }