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: #33690
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Author: addaleax

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:

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');
 
 letgetStringWidth;
 
-functiongetUserOptions(ctx){
-return{
+functiongetUserOptions(ctx,isCrossContext){
+constret={
 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(constkeyofObjectKeys(ret)){
+if((typeofret[key]==='object'||typeofret[key]==='function')&&
+ret[key]!==null){
+deleteret[key];
+}
+}
+ret.stylize=ObjectSetPrototypeOf((value,flavour)=>{
+letstylized;
+try{
+stylized=`${ctx.stylize(value,flavour)}`;
+}catch{}
+
+if(typeofstylized!=='string')returnvalue;
+// `stylized` is a string as it should be, which is safe to pass along.
+returnstylized;
+},null);
+}
+
+returnret;
 }
 
 /**
@@ -728,7 +758,10 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
 // This makes sure the recurseTimes are reported as before while using
 // a counter internally.
 constdepth=ctx.depth===null ? null : ctx.depth-recurseTimes;
-constret=maybeCustom.call(context,depth,getUserOptions(ctx));
+constisCrossContext=
+proxy!==undefined||!(contextinstanceofObject);
+constret=FunctionPrototypeCall(
+maybeCustom,context,depth,getUserOptions(ctx,isCrossContext));
 // If the custom inspection method returned `this`, don't go into
 // infinite recursion.
 if(ret!==context){

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.
+consttarget=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(typeoftarget.ctx,'object');
+constobjectGraph=fullObjectGraph(target);
+assert(!objectGraph.has(Object));
+assert(!objectGraph.has(Function));
+}
+
+{
+// Subtest 2: Use a stylize function that returns a non-primitive.
+constoutput=util.inspect(target,{
+stylize: common.mustCall((str)=>{
+return{};
+})
+});
+assert.strictEqual(output,'[object Object]');
+assert.strictEqual(typeoftarget.ctx,'object');
+constobjectGraph=fullObjectGraph(target);
+assert(!objectGraph.has(Object));
+assert(!objectGraph.has(Function));
+}
+
+{
+// Subtest 3: Use a stylize function that throws an exception.
+constoutput=util.inspect(target,{
+stylize: common.mustCall((str)=>{
+thrownewError('oops');
+})
+});
+assert.strictEqual(output,'🐈');
+assert.strictEqual(typeoftarget.ctx,'object');
+constobjectGraph=fullObjectGraph(target);
+assert(!objectGraph.has(Object));
+assert(!objectGraph.has(Function));
+}
+
+functionfullObjectGraph(value){
+constgraph=newSet([value]);
+
+for(constentryofgraph){
+if((typeofentry!=='object'&&typeofentry!=='function')||
+entry===null){
+continue;
+}
+
+graph.add(Object.getPrototypeOf(entry));
+constdescriptors=Object.values(
+Object.getOwnPropertyDescriptors(entry));
+for(constdescriptorofdescriptors){
+graph.add(descriptor.value);
+graph.add(descriptor.set);
+graph.add(descriptor.get);
+}
+}
+
+returngraph;
+}
+
+// Consistency check.
+assert(fullObjectGraph(global).has(Function.prototype));
+}