From d96c76507fc718caaf36979104c7b3288b33d44b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 21 Nov 2019 14:18:41 +0100 Subject: [PATCH] util: fix inspection of errors with tampered name or stack property This makes sure that `util.inspect()` does not throw while inspecting errors that have the name or stack property set to a different type than string. Fixes: https://github.com/nodejs/node/issues/30572 PR-URL: https://github.com/nodejs/node/pull/30576 Reviewed-By: David Carlier Reviewed-By: Anto Aravinth Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- lib/internal/util/inspect.js | 6 +++--- test/parallel/test-util-inspect.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index d8e68cf806bf64..d16ecf7412f2e7 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -904,12 +904,12 @@ function getFunctionBase(value, constructor, tag) { } function formatError(err, constructor, tag, ctx) { - let stack = err.stack || ErrorPrototype.toString(err); + const name = err.name != null ? String(err.name) : 'Error'; + let len = name.length; + let stack = err.stack ? String(err.stack) : ErrorPrototype.toString(err); // A stack trace may contain arbitrary data. Only manipulate the output // for "regular errors" (errors that "look normal") for now. - const name = err.name || 'Error'; - let len = name.length; if (constructor === null || (name.endsWith('Error') && stack.startsWith(name) && diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 81360854b9cb10..fac05f72e2eecc 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -690,6 +690,35 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324'); ); } +// Tampered error stack or name property (different type than string). +// Note: Symbols are not supported by `Error#toString()` which is called by +// accessing the `stack` property. +[ + [404, '404: foo', '[404]'], + [0, '0: foo', '[RangeError: foo]'], + [0n, '0: foo', '[RangeError: foo]'], + [null, 'null: foo', '[RangeError: foo]'], + [undefined, 'RangeError: foo', '[RangeError: foo]'], + [false, 'false: foo', '[RangeError: foo]'], + ['', 'foo', '[RangeError: foo]'], + [[1, 2, 3], '1,2,3: foo', '[1,2,3]'], +].forEach(([value, outputStart, stack]) => { + let err = new RangeError('foo'); + err.name = value; + assert( + util.inspect(err).startsWith(outputStart), + util.format( + 'The name set to %o did not result in the expected output "%s"', + value, + outputStart + ) + ); + + err = new RangeError('foo'); + err.stack = value; + assert.strictEqual(util.inspect(err), stack); +}); + // https://github.com/nodejs/node-v0.x-archive/issues/1941 assert.strictEqual(util.inspect(Object.create(Date.prototype)), 'Date {}');