From 80e3ef38eec99b1574a5555d4f3a60cf1a96920c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 16 Dec 2024 23:33:08 +0100 Subject: [PATCH] util: harden more built-in classes against 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/56225 Reviewed-By: Jordan Harband Reviewed-By: Vinícius Lourenço Claro Cardoso --- lib/buffer.js | 10 +++++++- lib/internal/util/inspect.js | 20 ++++++++++++--- test/parallel/test-util-inspect.js | 41 ++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 88625299bced5e..22bbb95eaf79e6 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -35,6 +35,7 @@ const { NumberMIN_SAFE_INTEGER, ObjectDefineProperties, ObjectDefineProperty, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, RegExpPrototypeSymbolReplace, StringPrototypeCharCodeAt, @@ -911,7 +912,14 @@ Buffer.prototype[customInspectSymbol] = function inspect(recurseTimes, ctx) { }), 27, -2); } } - return `<${this.constructor.name} ${str}>`; + let constructorName = 'Buffer'; + try { + const { constructor } = this; + if (typeof constructor === 'function' && ObjectPrototypeHasOwnProperty(constructor, 'name')) { + constructorName = constructor.name; + } + } catch { /* Ignore error and use default name */ } + return `<${constructorName} ${str}>`; }; Buffer.prototype.inspect = Buffer.prototype[customInspectSymbol]; diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 88bb084fb7fa41..cc76bbeed9bc11 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -2,7 +2,10 @@ const { Array, + ArrayBuffer, + ArrayBufferPrototype, ArrayIsArray, + ArrayPrototype, ArrayPrototypeFilter, ArrayPrototypeForEach, ArrayPrototypeIncludes, @@ -29,6 +32,8 @@ const { FunctionPrototypeSymbolHasInstance, FunctionPrototypeToString, JSONStringify, + Map, + MapPrototype, MapPrototypeEntries, MapPrototypeGetSize, MathFloor, @@ -68,6 +73,8 @@ const { SafeMap, SafeSet, SafeStringIterator, + Set, + SetPrototype, SetPrototypeGetSize, SetPrototypeValues, String, @@ -93,6 +100,8 @@ const { SymbolPrototypeValueOf, SymbolToPrimitive, SymbolToStringTag, + TypedArray, + TypedArrayPrototype, TypedArrayPrototypeGetLength, TypedArrayPrototypeGetSymbolToStringTag, Uint8Array, @@ -599,8 +608,13 @@ function isInstanceof(object, proto) { // Special-case for some builtin prototypes in case their `constructor` property has been tampered. const wellKnownPrototypes = new SafeMap(); -wellKnownPrototypes.set(ObjectPrototype, { name: 'Object', constructor: Object }); +wellKnownPrototypes.set(ArrayPrototype, { name: 'Array', constructor: Array }); +wellKnownPrototypes.set(ArrayBufferPrototype, { name: 'ArrayBuffer', constructor: ArrayBuffer }); wellKnownPrototypes.set(FunctionPrototype, { name: 'Function', constructor: Function }); +wellKnownPrototypes.set(MapPrototype, { name: 'Map', constructor: Map }); +wellKnownPrototypes.set(ObjectPrototype, { name: 'Object', constructor: Object }); +wellKnownPrototypes.set(SetPrototype, { name: 'Set', constructor: Set }); +wellKnownPrototypes.set(TypedArrayPrototype, { name: 'TypedArray', constructor: TypedArray }); function getConstructorName(obj, ctx, recurseTimes, protoProps) { let firstProto; @@ -825,12 +839,12 @@ function formatValue(ctx, value, recurseTimes, typedArray) { // Filter out the util module, its inspect function is special. maybeCustom !== inspect && // Also filter out any prototype objects using the circular check. - !(value.constructor && value.constructor.prototype === value)) { + ObjectGetOwnPropertyDescriptor(value, 'constructor')?.value?.prototype !== value) { // This makes sure the recurseTimes are reported as before while using // a counter internally. const depth = ctx.depth === null ? null : ctx.depth - recurseTimes; const isCrossContext = - proxy !== undefined || !(context instanceof Object); + proxy !== undefined || !FunctionPrototypeSymbolHasInstance(Object, context); const ret = FunctionPrototypeCall( maybeCustom, context, diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 8c8b99ffc3839c..94064d9491894c 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -3385,3 +3385,44 @@ assert.strictEqual( ); Object.defineProperty(BuiltinPrototype, 'constructor', desc); } +{ + const prototypes = [ + Array.prototype, + ArrayBuffer.prototype, + Buffer.prototype, + Function.prototype, + Map.prototype, + Object.prototype, + Reflect.getPrototypeOf(Uint8Array.prototype), + Set.prototype, + Uint8Array.prototype, + ]; + const descriptors = new Map(); + const buffer = Buffer.from('Hello'); + const o = { + arrayBuffer: new ArrayBuffer(), buffer, typedArray: Uint8Array.from(buffer), + array: [], func() {}, set: new Set([1]), map: new Map(), + }; + for (const BuiltinPrototype of prototypes) { + descriptors.set(BuiltinPrototype, Reflect.getOwnPropertyDescriptor(BuiltinPrototype, 'constructor')); + Object.defineProperty(BuiltinPrototype, 'constructor', { + get: () => BuiltinPrototype, + configurable: true, + }); + } + assert.strictEqual( + util.inspect(o), + '{\n' + + ' arrayBuffer: ArrayBuffer { [Uint8Contents]: <>, byteLength: 0 },\n' + + ' buffer: ,\n' + + ' typedArray: TypedArray(5) [Uint8Array] [ 72, 101, 108, 108, 111 ],\n' + + ' array: [],\n' + + ' func: [Function: func],\n' + + ' set: Set(1) { 1 },\n' + + ' map: Map(0) {}\n' + + '}', + ); + for (const [BuiltinPrototype, desc] of descriptors) { + Object.defineProperty(BuiltinPrototype, 'constructor', desc); + } +}