Skip to content

Commit

Permalink
util: harden more built-in classes against prototype pollution
Browse files Browse the repository at this point in the history
PR-URL: #56225
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
  • Loading branch information
aduh95 authored and ruyadorno committed Dec 20, 2024
1 parent 637932f commit 24a6659
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
10 changes: 9 additions & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const {
NumberMIN_SAFE_INTEGER,
ObjectDefineProperties,
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
ObjectSetPrototypeOf,
RegExpPrototypeSymbolReplace,
StringPrototypeCharCodeAt,
Expand Down Expand Up @@ -910,7 +911,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];

Expand Down
20 changes: 17 additions & 3 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

const {
Array,
ArrayBuffer,
ArrayBufferPrototype,
ArrayIsArray,
ArrayPrototype,
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
Expand All @@ -29,6 +32,8 @@ const {
FunctionPrototypeSymbolHasInstance,
FunctionPrototypeToString,
JSONStringify,
Map,
MapPrototype,
MapPrototypeEntries,
MapPrototypeGetSize,
MathFloor,
Expand Down Expand Up @@ -68,6 +73,8 @@ const {
SafeMap,
SafeSet,
SafeStringIterator,
Set,
SetPrototype,
SetPrototypeGetSize,
SetPrototypeValues,
String,
Expand All @@ -93,6 +100,8 @@ const {
SymbolPrototypeValueOf,
SymbolToPrimitive,
SymbolToStringTag,
TypedArray,
TypedArrayPrototype,
TypedArrayPrototypeGetLength,
TypedArrayPrototypeGetSymbolToStringTag,
Uint8Array,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
41 changes: 41 additions & 0 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -3353,3 +3353,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: <Buffer 48 65 6c 6c 6f>,\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);
}
}

0 comments on commit 24a6659

Please sign in to comment.