Repository: nodejs/node
Pull request: util: harden more built-in classes against prototype pollution

For an arbitrary list of classes, we can protect against userland mutation of the `constructor` prototype property, and speed up the `inspect` call by avoiding a `ObjectGetOwnPropertyDescriptor` call. I tried to come up with a list of classes that are the most likely to be often inspected, though I haven't used any rigorous method, I just came up with it, and I'm certainly open to add/remove some.

Refs: https://github.com/nodejs/node/pull/56188#discussion_r1876800640

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/buffer.js
patch: @@ -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];
 

filename: lib/internal/util/inspect.js
patch: @@ -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,

filename: test/parallel/test-util-inspect.js
patch: @@ -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: <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);
+  }
+}