From ec5b06eae385700391ea1e6e56e751978f097212 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 26 Jan 2021 16:52:31 +0100
Subject: [PATCH] util: fix infinite recursion during inspection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A specially crafted object with circular structures behind getters
could cause a infinite recursion. This is now fixed by detecting the
objects as already visited.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: https://github.com/nodejs/node/pull/37079
Fixes: https://github.com/nodejs/node/issues/37054
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
---
 lib/internal/util/inspect.js                  |  3 +
 ...est-util-inspect-getters-accessing-this.js | 67 ++++++++++++++-----
 2 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index dd9ef50228a296..73cac2caa51b8d 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -5,6 +5,7 @@ const {
   ArrayIsArray,
   ArrayPrototypeFilter,
   ArrayPrototypeForEach,
+  ArrayPrototypePop,
   ArrayPrototypePush,
   ArrayPrototypePushApply,
   ArrayPrototypeSort,
@@ -620,6 +621,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
     }
     // Get all own property names and symbols.
     keys = ReflectOwnKeys(obj);
+    ArrayPrototypePush(ctx.seen, main);
     for (const key of keys) {
       // Ignore the `constructor` property and keys that exist on layers above.
       if (key === 'constructor' ||
@@ -640,6 +642,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
         ArrayPrototypePush(output, value);
       }
     }
+    ArrayPrototypePop(ctx.seen);
   // Limit the inspection to up to three prototype layers. Using `recurseTimes`
   // is not a good choice here, because it's as if the properties are declared
   // on the current object from the users perspective.
diff --git a/test/parallel/test-util-inspect-getters-accessing-this.js b/test/parallel/test-util-inspect-getters-accessing-this.js
index 3d185b134e852d..998cd82db8f4b3 100644
--- a/test/parallel/test-util-inspect-getters-accessing-this.js
+++ b/test/parallel/test-util-inspect-getters-accessing-this.js
@@ -7,24 +7,61 @@ require('../common');
 
 const assert = require('assert');
 
-const util = require('util');
+const { inspect } = require('util');
 
-class X {
-  constructor() {
-    this._y = 123;
-  }
+{
+  class X {
+    constructor() {
+      this._y = 123;
+    }
 
-  get y() {
-    return this._y;
+    get y() {
+      return this._y;
+    }
   }
+
+  const result = inspect(new X(), {
+    getters: true,
+    showHidden: true
+  });
+
+  assert.strictEqual(
+    result,
+    'X { _y: 123, [y]: [Getter: 123] }'
+  );
 }
 
-const result = util.inspect(new X(), {
-  getters: true,
-  showHidden: true
-});
+// Regression test for https://github.com/nodejs/node/issues/37054
+{
+  class A {
+    constructor(B) {
+      this.B = B;
+    }
+    get b() {
+      return this.B;
+    }
+  }
+
+  class B {
+    constructor() {
+      this.A = new A(this);
+    }
+    get a() {
+      return this.A;
+    }
+  }
+
+  const result = inspect(new B(), {
+    depth: 1,
+    getters: true,
+    showHidden: true
+  });
 
-assert.strictEqual(
-  result,
-  'X { _y: 123, [y]: [Getter: 123] }'
-);
+  assert.strictEqual(
+    result,
+    '<ref *1> B {\n' +
+    '  A: A { B: [Circular *1], [b]: [Getter] [Circular *1] },\n' +
+    '  [a]: [Getter] A { B: [Circular *1], [b]: [Getter] [Circular *1] }\n' +
+    '}',
+  );
+}