repl: improve robustness wrt to prototype pollution

PR-URL: https://github.com/nodejs/node/pull/45604
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/debugger/inspect_repl.js

@@ -28,14 +28,15 @@ const {
   ReflectGetOwnPropertyDescriptor,
   ReflectOwnKeys,
   RegExpPrototypeExec,
-  RegExpPrototypeSymbolReplace,
   SafeMap,
-  SafePromiseAll,
+  SafePromiseAllReturnArrayLike,
+  SafePromiseAllReturnVoid,
   String,
   StringFromCharCode,
   StringPrototypeEndsWith,
   StringPrototypeIncludes,
   StringPrototypeRepeat,
+  StringPrototypeReplaceAll,
   StringPrototypeSlice,
   StringPrototypeSplit,
   StringPrototypeStartsWith,
@@ -53,7 +54,7 @@ const Repl = require('repl');
 const vm = require('vm');
 const { fileURLToPath } = require('internal/url');
 
-const { customInspectSymbol } = require('internal/util');
+const { customInspectSymbol, SideEffectFreeRegExpPrototypeSymbolReplace } = require('internal/util');
 const { inspect: utilInspect } = require('internal/util/inspect');
 const debuglog = require('internal/util/debuglog').debuglog('inspect');
 
@@ -121,7 +122,7 @@ const {
 } = internalBinding('builtins');
 const NATIVES = internalBinding('natives');
 function isNativeUrl(url) {
-  url = RegExpPrototypeSymbolReplace(/\.js$/, url, '');
+  url = SideEffectFreeRegExpPrototypeSymbolReplace(/\.js$/, url, '');
 
   return StringPrototypeStartsWith(url, 'node:internal/') ||
          ArrayPrototypeIncludes(PUBLIC_BUILTINS, url) ||
@@ -159,7 +160,7 @@ function markSourceColumn(sourceText, position, useColors) {
 
   // Colourize char if stdout supports colours
   if (useColors) {
-    tail = RegExpPrototypeSymbolReplace(/(.+?)([^\w]|$)/, tail,
+    tail = SideEffectFreeRegExpPrototypeSymbolReplace(/(.+?)([^\w]|$)/, tail,
                                                       '\u001b[32m$1\u001b[39m$2');
   }
 
@@ -340,7 +341,7 @@ class ScopeSnapshot {
                  StringPrototypeSlice(this.type, 1);
     const name = this.name ? `<${this.name}>` : '';
     const prefix = `${type}${name} `;
-    return RegExpPrototypeSymbolReplace(/^Map /,
+    return SideEffectFreeRegExpPrototypeSymbolReplace(/^Map /,
                                                       utilInspect(this.properties, opts),
                                                       prefix);
   }
@@ -517,7 +518,7 @@ function createRepl(inspector) {
     }
 
     loadScopes() {
-      return SafePromiseAll(
+      return SafePromiseAllReturnArrayLike(
         ArrayPrototypeFilter(
           this.scopeChain,
           (scope) => scope.type !== 'global'
@@ -656,14 +657,14 @@ function createRepl(inspector) {
                            (error) => `<${error.message}>`);
     const lastIndex = watchedExpressions.length - 1;
 
-    const values = await SafePromiseAll(watchedExpressions, inspectValue);
+    const values = await SafePromiseAllReturnArrayLike(watchedExpressions, inspectValue);
     const lines = ArrayPrototypeMap(watchedExpressions, (expr, idx) => {
       const prefix = `${leftPad(idx, ' ', lastIndex)}: ${expr} =`;
       const value = inspect(values[idx]);
       if (!StringPrototypeIncludes(value, '\n')) {
         return `${prefix} ${value}`;
       }
-      return `${prefix}\n    ${RegExpPrototypeSymbolReplace(/\n/g, value, '\n    ')}`;
+      return `${prefix}\n    ${StringPrototypeReplaceAll(value, '\n', '\n    ')}`;
     });
     const valueList = ArrayPrototypeJoin(lines, '\n');
     return verbose ? `Watchers:\n${valueList}\n` : valueList;
@@ -805,7 +806,7 @@ function createRepl(inspector) {
         registerBreakpoint);
     }
 
-    const escapedPath = RegExpPrototypeSymbolReplace(/([/\\.?*()^${}|[\]])/g,
+    const escapedPath = SideEffectFreeRegExpPrototypeSymbolReplace(/([/\\.?*()^${}|[\]])/g,
                                                                    script, '\\$1');
     const urlRegex = `^(.*[\\/\\\\])?${escapedPath}$`;
 
@@ -860,9 +861,9 @@ function createRepl(inspector) {
                                       location.lineNumber + 1));
     if (!newBreakpoints.length) return PromiseResolve();
     return PromisePrototypeThen(
-      SafePromiseAll(newBreakpoints),
+      SafePromiseAllReturnVoid(newBreakpoints),
-      (results) => {
+      () => {
-        print(`${results.length} breakpoints restored.`);
+        print(`${newBreakpoints.length} breakpoints restored.`);
       });
   }
 
@@ -896,7 +897,7 @@ function createRepl(inspector) {
 
     inspector.suspendReplWhile(() =>
       PromisePrototypeThen(
-        SafePromiseAll([formatWatchers(true), selectedFrame.list(2)]),
+        SafePromiseAllReturnArrayLike([formatWatchers(true), selectedFrame.list(2)]),
         ({ 0: watcherList, 1: context }) => {
           const breakContext = watcherList ?
             `${watcherList}\n${inspect(context)}` :

lib/internal/util.js

@@ -23,13 +23,24 @@ const {
   ReflectApply,
   ReflectConstruct,
   RegExpPrototypeExec,
+  RegExpPrototypeGetDotAll,
+  RegExpPrototypeGetGlobal,
+  RegExpPrototypeGetHasIndices,
+  RegExpPrototypeGetIgnoreCase,
+  RegExpPrototypeGetMultiline,
+  RegExpPrototypeGetSticky,
+  RegExpPrototypeGetUnicode,
+  RegExpPrototypeGetSource,
   SafeMap,
   SafeSet,
+  SafeWeakMap,
   StringPrototypeReplace,
   StringPrototypeToLowerCase,
   StringPrototypeToUpperCase,
   Symbol,
   SymbolFor,
+  SymbolReplace,
+  SymbolSplit,
 } = primordials;
 
 const {
@@ -646,6 +657,35 @@ function SideEffectFreeRegExpPrototypeExec(regex, string) {
   return FunctionPrototypeCall(RegExpFromAnotherRealm.prototype.exec, regex, string);
 }
 
+const crossRelmRegexes = new SafeWeakMap();
+function getCrossRelmRegex(regex) {
+  const cached = crossRelmRegexes.get(regex);
+  if (cached) return cached;
+
+  let flagString = '';
+  if (RegExpPrototypeGetHasIndices(regex)) flagString += 'd';
+  if (RegExpPrototypeGetGlobal(regex)) flagString += 'g';
+  if (RegExpPrototypeGetIgnoreCase(regex)) flagString += 'i';
+  if (RegExpPrototypeGetMultiline(regex)) flagString += 'm';
+  if (RegExpPrototypeGetDotAll(regex)) flagString += 's';
+  if (RegExpPrototypeGetUnicode(regex)) flagString += 'u';
+  if (RegExpPrototypeGetSticky(regex)) flagString += 'y';
+
+  const { RegExp: RegExpFromAnotherRealm } = getInternalGlobal();
+  const crossRelmRegex = new RegExpFromAnotherRealm(RegExpPrototypeGetSource(regex), flagString);
+  crossRelmRegexes.set(regex, crossRelmRegex);
+  return crossRelmRegex;
+}
+
+function SideEffectFreeRegExpPrototypeSymbolReplace(regex, string, replacement) {
+  return getCrossRelmRegex(regex)[SymbolReplace](string, replacement);
+}
+
+function SideEffectFreeRegExpPrototypeSymbolSplit(regex, string, limit = undefined) {
+  return getCrossRelmRegex(regex)[SymbolSplit](string, limit);
+}
+
+
 function isArrayBufferDetached(value) {
   if (ArrayBufferPrototypeGetByteLength(value) === 0) {
     return _isArrayBufferDetached(value);
@@ -684,6 +724,8 @@ module.exports = {
   once,
   promisify,
   SideEffectFreeRegExpPrototypeExec,
+  SideEffectFreeRegExpPrototypeSymbolReplace,
+  SideEffectFreeRegExpPrototypeSymbolSplit,
   sleep,
   spliceOne,
   toUSVString,

lib/repl.js

@@ -77,8 +77,6 @@ const {
   ReflectApply,
   RegExp,
   RegExpPrototypeExec,
-  RegExpPrototypeSymbolReplace,
-  RegExpPrototypeSymbolSplit,
   SafePromiseRace,
   SafeSet,
   SafeWeakSet,
@@ -111,7 +109,9 @@ const {
 const {
   decorateErrorStack,
   isError,
-  deprecate
+  deprecate,
+  SideEffectFreeRegExpPrototypeSymbolReplace,
+  SideEffectFreeRegExpPrototypeSymbolSplit,
 } = require('internal/util');
 const { inspect } = require('internal/util/inspect');
 const vm = require('vm');
@@ -455,7 +455,7 @@ function REPLServer(prompt,
 
           // Remove all "await"s and attempt running the script
           // in order to detect if error is truly non recoverable
-          const fallbackCode = RegExpPrototypeSymbolReplace(/\bawait\b/g, code, '');
+          const fallbackCode = SideEffectFreeRegExpPrototypeSymbolReplace(/\bawait\b/g, code, '');
           try {
             vm.createScript(fallbackCode, {
               filename: file,
@@ -680,22 +680,22 @@ function REPLServer(prompt,
         if (e.stack) {
           if (e.name === 'SyntaxError') {
             // Remove stack trace.
-            e.stack = RegExpPrototypeSymbolReplace(
+            e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
               /^\s+at\s.*\n?/gm,
-              RegExpPrototypeSymbolReplace(/^REPL\d+:\d+\r?\n/, e.stack, ''),
+              SideEffectFreeRegExpPrototypeSymbolReplace(/^REPL\d+:\d+\r?\n/, e.stack, ''),
               '');
             const importErrorStr = 'Cannot use import statement outside a ' +
               'module';
             if (StringPrototypeIncludes(e.message, importErrorStr)) {
               e.message = 'Cannot use import statement inside the Node.js ' +
                 'REPL, alternatively use dynamic import';
-              e.stack = RegExpPrototypeSymbolReplace(
+              e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
                 /SyntaxError:.*\n/,
                 e.stack,
                 `SyntaxError: ${e.message}\n`);
             }
           } else if (self.replMode === module.exports.REPL_MODE_STRICT) {
-            e.stack = RegExpPrototypeSymbolReplace(
+            e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
               /(\s+at\s+REPL\d+:)(\d+)/,
               e.stack,
               (_, pre, line) => pre + (line - 1)
@@ -727,7 +727,7 @@ function REPLServer(prompt,
       if (errStack === '') {
         errStack = self.writer(e);
       }
-      const lines = RegExpPrototypeSymbolSplit(/(?<=\n)/, errStack);
+      const lines = SideEffectFreeRegExpPrototypeSymbolSplit(/(?<=\n)/, errStack);
       let matched = false;
 
       errStack = '';

test/parallel/test-primordials-regexp.js

@@ -5,6 +5,7 @@ const { mustNotCall } = require('../common');
 const assert = require('assert');
 
 const {
+  RegExpPrototypeExec,
   RegExpPrototypeSymbolReplace,
   RegExpPrototypeSymbolSearch,
   RegExpPrototypeSymbolSplit,
@@ -12,6 +13,12 @@ const {
   hardenRegExp,
 } = require('internal/test/binding').primordials;
 
+const {
+  SideEffectFreeRegExpPrototypeExec,
+  SideEffectFreeRegExpPrototypeSymbolReplace,
+  SideEffectFreeRegExpPrototypeSymbolSplit,
+} = require('internal/util');
+
 
 Object.defineProperties(RegExp.prototype, {
   [Symbol.match]: {
@@ -89,14 +96,46 @@ hardenRegExp(hardenRegExp(/1/));
 // IMO there are no valid use cases in node core to use RegExpPrototypeSymbolMatch
 // or RegExpPrototypeSymbolMatchAll, they are inherently unsafe.
 
+assert.strictEqual(RegExpPrototypeExec(/foo/, 'bar'), null);
+assert.strictEqual(RegExpPrototypeExec(hardenRegExp(/foo/), 'bar'), null);
+assert.strictEqual(SideEffectFreeRegExpPrototypeExec(/foo/, 'bar'), null);
+assert.strictEqual(SideEffectFreeRegExpPrototypeExec(hardenRegExp(/foo/), 'bar'), null);
+{
+  const expected = ['bar'];
+  Object.defineProperties(expected, {
+    index: { __proto__: null, configurable: true, writable: true, enumerable: true, value: 0 },
+    input: { __proto__: null, configurable: true, writable: true, enumerable: true, value: 'bar' },
+    groups: { __proto__: null, configurable: true, writable: true, enumerable: true },
+  });
+  const actual = SideEffectFreeRegExpPrototypeExec(/bar/, 'bar');
+
+  // assert.deepStrictEqual(actual, expected) doesn't work for cross-realm comparison.
+
+  assert.strictEqual(Array.isArray(actual), Array.isArray(expected));
+  assert.deepStrictEqual(Reflect.ownKeys(actual), Reflect.ownKeys(expected));
+  for (const key of Reflect.ownKeys(expected)) {
+    assert.deepStrictEqual(
+      Reflect.getOwnPropertyDescriptor(actual, key),
+      Reflect.getOwnPropertyDescriptor(expected, key),
+    );
+  }
+}
 {
   const myRegex = hardenRegExp(/a/);
   assert.strictEqual(RegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'bear');
 }
+{
+  const myRegex = /a/;
+  assert.strictEqual(SideEffectFreeRegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'bear');
+}
 {
   const myRegex = hardenRegExp(/a/g);
   assert.strictEqual(RegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'beer');
 }
+{
+  const myRegex = /a/g;
+  assert.strictEqual(SideEffectFreeRegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'beer');
+}
 {
   const myRegex = hardenRegExp(/a/);
   assert.strictEqual(RegExpPrototypeSymbolSearch(myRegex, 'baar'), 1);
@@ -109,6 +148,22 @@ hardenRegExp(hardenRegExp(/1/));
   const myRegex = hardenRegExp(/a/);
   assert.deepStrictEqual(RegExpPrototypeSymbolSplit(myRegex, 'baar', 0), []);
 }
+{
+  const myRegex = /a/;
+  const expected = [];
+  const actual = SideEffectFreeRegExpPrototypeSymbolSplit(myRegex, 'baar', 0);
+
+  // assert.deepStrictEqual(actual, expected) doesn't work for cross-realm comparison.
+
+  assert.strictEqual(Array.isArray(actual), Array.isArray(expected));
+  assert.deepStrictEqual(Reflect.ownKeys(actual), Reflect.ownKeys(expected));
+  for (const key of Reflect.ownKeys(expected)) {
+    assert.deepStrictEqual(
+      Reflect.getOwnPropertyDescriptor(actual, key),
+      Reflect.getOwnPropertyDescriptor(expected, key),
+    );
+  }
+}
 {
   const myRegex = hardenRegExp(/a/);
   assert.deepStrictEqual(RegExpPrototypeSymbolSplit(myRegex, 'baar', 1), ['b']);

test/parallel/test-startup-empty-regexp-statics.js

@@ -40,6 +40,23 @@ const allRegExpStatics =
   assert.strictEqual(child.signal, null);
 }
 
+{
+  const child = spawnSync(process.execPath,
+                          [ '--expose-internals', '-p', `const {
+                            SideEffectFreeRegExpPrototypeExec,
+                            SideEffectFreeRegExpPrototypeSymbolReplace,
+                            SideEffectFreeRegExpPrototypeSymbolSplit,
+                          } = require("internal/util");
+                          SideEffectFreeRegExpPrototypeExec(/foo/, "foo");
+                          SideEffectFreeRegExpPrototypeSymbolReplace(/o/, "foo", "a");
+                          SideEffectFreeRegExpPrototypeSymbolSplit(/o/, "foo");
+                          ${allRegExpStatics}` ],
+                          { stdio: ['inherit', 'pipe', 'inherit'] });
+  assert.match(child.stdout.toString(), /^undefined\r?\n$/);
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
+}
+
 {
   const child = spawnSync(process.execPath,
                           [ '-e', `console.log(${allRegExpStatics})`, '--input-type=module' ],