From e0206bafe648e5ec56ca017b0861ff7cd0c47ee6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 2 Jun 2020 00:42:46 +0200 Subject: [PATCH] util: restrict custom inspect function + vm.Context interaction When `util.inspect()` is called on an object with a custom inspect function, and that object is from a different `vm.Context`, that function will not receive any arguments that access context-specific data anymore. PR-URL: https://github.com/nodejs/node/pull/33690 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- doc/api/util.md | 7 +++ lib/internal/util/inspect.js | 39 ++++++++++++-- test/parallel/test-util-inspect.js | 83 ++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 3 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 269210c1a38f07..7b3d08e0a0bcab 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -398,6 +398,13 @@ stream.write('With ES6'); added: v0.3.0 changes: - version: v14.0.0 + pr-url: https://github.com/nodejs/node/pull/33690 + description: If `object` is from a different `vm.Context` now, a custom + inspection function on it will not receive context-specific + arguments anymore. + - version: + - v13.13.0 + - v12.17.0 pr-url: https://github.com/nodejs/node/pull/32392 description: The `maxStringLength` option is supported now. - version: diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 4cb0ee55f080e7..03e62226236df3 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -12,6 +12,7 @@ const { DatePrototypeToString, ErrorPrototypeToString, Float32Array, + FunctionPrototypeCall, FunctionPrototypeToString, Int16Array, JSONStringify, @@ -26,6 +27,7 @@ const { Number, NumberIsNaN, NumberPrototypeValueOf, + Object, ObjectAssign, ObjectCreate, ObjectDefineProperty, @@ -38,6 +40,7 @@ const { ObjectPrototypeHasOwnProperty, ObjectPrototypePropertyIsEnumerable, ObjectSeal, + ObjectSetPrototypeOf, RegExp, RegExpPrototypeToString, Set, @@ -213,8 +216,8 @@ const ansi = new RegExp(ansiPattern, 'g'); let getStringWidth; -function getUserOptions(ctx) { - return { +function getUserOptions(ctx, isCrossContext) { + const ret = { stylize: ctx.stylize, showHidden: ctx.showHidden, depth: ctx.depth, @@ -229,6 +232,33 @@ function getUserOptions(ctx) { getters: ctx.getters, ...ctx.userOptions }; + + // Typically, the target value will be an instance of `Object`. If that is + // *not* the case, the object may come from another vm.Context, and we want + // to avoid passing it objects from this Context in that case, so we remove + // the prototype from the returned object itself + the `stylize()` function, + // and remove all other non-primitives, including non-primitive user options. + if (isCrossContext) { + ObjectSetPrototypeOf(ret, null); + for (const key of ObjectKeys(ret)) { + if ((typeof ret[key] === 'object' || typeof ret[key] === 'function') && + ret[key] !== null) { + delete ret[key]; + } + } + ret.stylize = ObjectSetPrototypeOf((value, flavour) => { + let stylized; + try { + stylized = `${ctx.stylize(value, flavour)}`; + } catch {} + + if (typeof stylized !== 'string') return value; + // `stylized` is a string as it should be, which is safe to pass along. + return stylized; + }, null); + } + + return ret; } /** @@ -728,7 +758,10 @@ function formatValue(ctx, value, recurseTimes, typedArray) { // This makes sure the recurseTimes are reported as before while using // a counter internally. const depth = ctx.depth === null ? null : ctx.depth - recurseTimes; - const ret = maybeCustom.call(context, depth, getUserOptions(ctx)); + const isCrossContext = + proxy !== undefined || !(context instanceof Object); + const ret = FunctionPrototypeCall( + maybeCustom, context, depth, getUserOptions(ctx, isCrossContext)); // If the custom inspection method returned `this`, don't go into // infinite recursion. if (ret !== context) { diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 1a38904e09e670..55dcb851154ecf 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -2906,3 +2906,86 @@ assert.strictEqual( "'aaaa'... 999996 more characters" ); } + +{ + // Verify that util.inspect() invokes custom inspect functions on objects + // from other vm.Contexts but does not pass data from its own Context to that + // function. + const target = vm.runInNewContext(` + ({ + [Symbol.for('nodejs.util.inspect.custom')](depth, ctx) { + this.depth = depth; + this.ctx = ctx; + try { + this.stylized = ctx.stylize('🐈'); + } catch (e) { + this.stylizeException = e; + } + return this.stylized; + } + }) + `, Object.create(null)); + assert.strictEqual(target.ctx, undefined); + + { + // Subtest 1: Just try to inspect the object with default options. + assert.strictEqual(util.inspect(target), '🐈'); + assert.strictEqual(typeof target.ctx, 'object'); + const objectGraph = fullObjectGraph(target); + assert(!objectGraph.has(Object)); + assert(!objectGraph.has(Function)); + } + + { + // Subtest 2: Use a stylize function that returns a non-primitive. + const output = util.inspect(target, { + stylize: common.mustCall((str) => { + return {}; + }) + }); + assert.strictEqual(output, '[object Object]'); + assert.strictEqual(typeof target.ctx, 'object'); + const objectGraph = fullObjectGraph(target); + assert(!objectGraph.has(Object)); + assert(!objectGraph.has(Function)); + } + + { + // Subtest 3: Use a stylize function that throws an exception. + const output = util.inspect(target, { + stylize: common.mustCall((str) => { + throw new Error('oops'); + }) + }); + assert.strictEqual(output, '🐈'); + assert.strictEqual(typeof target.ctx, 'object'); + const objectGraph = fullObjectGraph(target); + assert(!objectGraph.has(Object)); + assert(!objectGraph.has(Function)); + } + + function fullObjectGraph(value) { + const graph = new Set([value]); + + for (const entry of graph) { + if ((typeof entry !== 'object' && typeof entry !== 'function') || + entry === null) { + continue; + } + + graph.add(Object.getPrototypeOf(entry)); + const descriptors = Object.values( + Object.getOwnPropertyDescriptors(entry)); + for (const descriptor of descriptors) { + graph.add(descriptor.value); + graph.add(descriptor.set); + graph.add(descriptor.get); + } + } + + return graph; + } + + // Consistency check. + assert(fullObjectGraph(global).has(Function.prototype)); +}