Repository: nodejs/node
Pull request: util: use more defensive code when inspecting error objects

Fixes: https://github.com/nodejs/node/issues/60107

<!--
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/internal/util.js
patch: @@ -8,6 +8,7 @@ const {
   Error,
   ErrorCaptureStackTrace,
   FunctionPrototypeCall,
+  FunctionPrototypeSymbolHasInstance,
   NumberParseInt,
   ObjectDefineProperties,
   ObjectDefineProperty,
@@ -96,7 +97,7 @@ function isError(e) {
   // An error could be an instance of Error while not being a native error
   // or could be from a different realm and not be instance of Error but still
   // be a native error.
-  return isNativeError(e) || e instanceof Error;
+  return isNativeError(e) || FunctionPrototypeSymbolHasInstance(Error, e);
 }
 
 // Keep a list of deprecation codes that have been warned on so we only warn on

filename: lib/internal/util/inspect.js
patch: @@ -72,6 +72,7 @@ const {
   ObjectPrototype,
   ObjectPrototypeHasOwnProperty,
   ObjectPrototypePropertyIsEnumerable,
+  ObjectPrototypeToString,
   ObjectSeal,
   ObjectSetPrototypeOf,
   Promise,
@@ -1720,13 +1721,19 @@ function getDuplicateErrorFrameRanges(frames) {
 }
 
 function getStackString(ctx, error) {
-  if (error.stack) {
-    if (typeof error.stack === 'string') {
-      return error.stack;
+  let stack;
+  try {
+    stack = error.stack;
+  } catch {
+    // If stack is getter that throws, we ignore the error.
+  }
+  if (stack) {
+    if (typeof stack === 'string') {
+      return stack;
     }
     ctx.seen.push(error);
     ctx.indentationLvl += 4;
-    const result = formatValue(ctx, error.stack);
+    const result = formatValue(ctx, stack);
     ctx.indentationLvl -= 4;
     ctx.seen.pop();
     return `${ErrorPrototypeToString(error)}\n    ${result}`;
@@ -1822,18 +1829,6 @@ function improveStack(stack, constructor, name, tag) {
   return stack;
 }
 
-function removeDuplicateErrorKeys(ctx, keys, err, stack) {
-  if (!ctx.showHidden && keys.length !== 0) {
-    for (const name of ['name', 'message', 'stack']) {
-      const index = ArrayPrototypeIndexOf(keys, name);
-      // Only hide the property if it's a string and if it's part of the original stack
-      if (index !== -1 && (typeof err[name] !== 'string' || StringPrototypeIncludes(stack, err[name]))) {
-        ArrayPrototypeSplice(keys, index, 1);
-      }
-    }
-  }
-}
-
 function markNodeModules(ctx, line) {
   let tempLine = '';
   let lastPos = 0;
@@ -1916,28 +1911,72 @@ function safeGetCWD() {
 }
 
 function formatError(err, constructor, tag, ctx, keys) {
-  const name = err.name != null ? err.name : 'Error';
-  let stack = getStackString(ctx, err);
+  let message, name, stack;
+  try {
+    stack = getStackString(ctx, err);
+  } catch {
+    return ObjectPrototypeToString(err);
+  }
 
-  removeDuplicateErrorKeys(ctx, keys, err, stack);
+  let messageIsGetterThatThrows = false;
+  try {
+    message = err.message;
+  } catch {
+    messageIsGetterThatThrows = true;
+  }
+  let nameIsGetterThatThrows = false;
+  try {
+    name = err.name;
+  } catch {
+    nameIsGetterThatThrows = true;
+  }
+
+  if (!ctx.showHidden && keys.length !== 0) {
+    const index = ArrayPrototypeIndexOf(keys, 'stack');
+    if (index !== -1) {
+      ArrayPrototypeSplice(keys, index, 1);
+    }
+
+    if (!messageIsGetterThatThrows) {
+      const index = ArrayPrototypeIndexOf(keys, 'message');
+      // Only hide the property if it's a string and if it's part of the original stack
+      if (index !== -1 && (typeof message !== 'string' || StringPrototypeIncludes(stack, message))) {
+        ArrayPrototypeSplice(keys, index, 1);
+      }
+    }
+
+    if (!nameIsGetterThatThrows) {
+      const index = ArrayPrototypeIndexOf(keys, 'name');
+      // Only hide the property if it's a string and if it's part of the original stack
+      if (index !== -1 && (typeof name !== 'string' || StringPrototypeIncludes(stack, name))) {
+        ArrayPrototypeSplice(keys, index, 1);
+      }
+    }
+  }
+  name ??= 'Error';
 
   if ('cause' in err &&
       (keys.length === 0 || !ArrayPrototypeIncludes(keys, 'cause'))) {
     ArrayPrototypePush(keys, 'cause');
   }
 
   // Print errors aggregated into AggregateError
-  if (ArrayIsArray(err.errors) &&
+  try {
+    const errors = err.errors;
+    if (ArrayIsArray(errors) &&
       (keys.length === 0 || !ArrayPrototypeIncludes(keys, 'errors'))) {
-    ArrayPrototypePush(keys, 'errors');
+      ArrayPrototypePush(keys, 'errors');
+    }
+  } catch {
+    // If errors is a getter that throws, we ignore the error.
   }
 
   stack = improveStack(stack, constructor, name, tag);
 
   // Ignore the error message if it's contained in the stack.
-  let pos = (err.message && StringPrototypeIndexOf(stack, err.message)) || -1;
+  let pos = (message && StringPrototypeIndexOf(stack, message)) || -1;
   if (pos !== -1)
-    pos += err.message.length;
+    pos += message.length;
   // Wrap the error in brackets in case it has no stack trace.
   const stackStart = StringPrototypeIndexOf(stack, '\n    at', pos);
   if (stackStart === -1) {

filename: test/parallel/test-util-inspect.js
patch: @@ -3716,3 +3716,28 @@ ${error.stack.split('\n').slice(1).join('\n')}`,
 
   assert.strictEqual(inspect(error), '[Error: foo\n    [Error: bar\n        [Circular *1]]]');
 }
+
+{
+  Object.defineProperty(Error, Symbol.hasInstance,
+                        { __proto__: null, value: common.mustNotCall(), configurable: true });
+  const error = new Error();
+
+  const throwingGetter = {
+    __proto__: null,
+    get() {
+      throw error;
+    },
+    configurable: true,
+    enumerable: true,
+  };
+
+  Object.defineProperties(error, {
+    name: throwingGetter,
+    stack: throwingGetter,
+    cause: throwingGetter,
+  });
+
+  assert.strictEqual(inspect(error), `[object Error] {\n  stack: [Getter/Setter],\n  name: [Getter],\n  cause: [Getter]\n}`);
+  assert.match(inspect(DOMException.prototype), /^\[object DOMException\] \{/);
+  delete Error[Symbol.hasInstance];
+}