From 78e354f8041fb822c748e1fa38a27e098b290e84 Mon Sep 17 00:00:00 2001 From: Christian Aurich Date: Thu, 27 Aug 2026 23:15:35 -0300 Subject: [PATCH] errors: validate constructor name determineSpecificType() checks for a usable constructor name with `'name' in value.constructor`. That accepts an empty name, and the `in` operator requires its right-hand side to be an object. Anonymous classes own a `name` that is the empty string, so they produce messages ending in a dangling "Received an instance of ". A truthy primitive `constructor` reaches the `in` operator and throws while the message is being built, so ERR_INVALID_ARG_TYPE is replaced by a TypeError carrying no `code`. The latter is reachable from untrusted input, since `constructor` is an ordinary JSON key. The check changed in #49696, while this function was rewritten as a switch. That pull request updated test/common's invalidArgTypeHelper to match its deliberate change to the `function` branch, but left the helper's `object` branch on the original truthy check, so the two have disagreed since. Read `constructor` and its `name` once and use the name only when it is a non-empty string. Requiring a string also stops meaningless names from being interpolated: 42 currently yields "an instance of 42", and a symbol name throws outright. The current check reads `constructor` three times, which an accessor can observe. Move the helper to the same check so the two cannot drift apart again. Signed-off-by: Christian Aurich --- lib/internal/errors.js | 11 +++- test/common/index.js | 5 +- .../test-error-value-type-detection.mjs | 55 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 438bde842d8b..40c9afac9a5d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1035,11 +1035,16 @@ function determineSpecificType(value) { return `type symbol (${String(value)})`; case 'function': return `function ${value.name}`; - case 'object': - if (value.constructor && 'name' in value.constructor) { - return `an instance of ${value.constructor.name}`; + case 'object': { + // `constructor` may be user-controlled: it need not be an object, and + // its `name` need not be a non-empty string. Reading either can invoke + // an accessor, so read each one once. + const name = value.constructor?.name; + if (typeof name === 'string' && name !== '') { + return `an instance of ${name}`; } return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`; + } case 'string': value.length > 28 && (value = `${StringPrototypeSlice(value, 0, 25)}...`); if (StringPrototypeIndexOf(value, "'") === -1) { diff --git a/test/common/index.js b/test/common/index.js index e37b354f8259..3ec677e566d8 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -855,8 +855,9 @@ function invalidArgTypeHelper(input) { return ` Received function ${input.name}`; } if (typeof input === 'object') { - if (input.constructor?.name) { - return ` Received an instance of ${input.constructor.name}`; + const name = input.constructor?.name; + if (typeof name === 'string' && name !== '') { + return ` Received an instance of ${name}`; } return ` Received ${inspect(input, { depth: -1 })}`; } diff --git a/test/parallel/test-error-value-type-detection.mjs b/test/parallel/test-error-value-type-detection.mjs index e44b19297a96..cb794514ed39 100644 --- a/test/parallel/test-error-value-type-detection.mjs +++ b/test/parallel/test-error-value-type-detection.mjs @@ -209,3 +209,58 @@ assert.strictEqual( determineSpecificType(new WeakSet()), 'an instance of WeakSet', ); + +// Anonymous classes have an empty `name`, so the value is inspected instead. +// `inspect(..., { depth: -1 })` prints this one as `{}` because it has no own +// properties; the values below carry a `constructor` property of their own and +// print as `[Object]`. +assert.strictEqual( + determineSpecificType(new (class {})()), + '{}', +); + +// `constructor` is an ordinary, user-controlled property that need not be a +// function. Describing such a value must not throw. +assert.strictEqual( + determineSpecificType(JSON.parse('{"constructor": 5}')), + '[Object]', +); + +assert.strictEqual( + determineSpecificType({ constructor: { name: '' } }), + '[Object]', +); + +// A `constructor.name` that is not a usable string must not be interpolated +// into the message. +assert.strictEqual( + determineSpecificType({ constructor: { name: Symbol('x') } }), + '[Object]', +); + +assert.strictEqual( + determineSpecificType({ constructor: { name: 42 } }), + '[Object]', +); + +// `constructor` and its `name` are each read once. Both may be accessors, so +// repeated reads are observable, and a `name` validated by one read and +// interpolated from another need not be the same value twice. +let constructorReads = 0; +let nameReads = 0; +const named = { + get constructor() { + constructorReads++; + return { get name() { nameReads++; return 'Foo'; } }; + }, +}; +assert.strictEqual(determineSpecificType(named), 'an instance of Foo'); +assert.strictEqual(constructorReads, 1); +assert.strictEqual(nameReads, 1); + +// Building the error must not fail when `constructor` is a truthy primitive. +assert.strictEqual( + new errorsModule.codes.ERR_INVALID_ARG_TYPE( + 'arg', 'string', JSON.parse('{"constructor": 5}')).code, + 'ERR_INVALID_ARG_TYPE', +);