Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 784ed59

Browse filesBrowse files
aduh95juanarbol
aduh95
authored and
juanarbol
committed
repl: improve robustness wrt to prototype pollution
PR-URL: #45604 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e0cda56 commit 784ed59
Copy full SHA for 784ed59

File tree

Expand file treeCollapse file tree

5 files changed

+142
-27
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+142
-27
lines changed
Open diff view settings
Collapse file

‎lib/internal/debugger/inspect_repl.js‎

Copy file name to clipboardExpand all lines: lib/internal/debugger/inspect_repl.js
+19-18Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@ const {
2828
ReflectGetOwnPropertyDescriptor,
2929
ReflectOwnKeys,
3030
RegExpPrototypeExec,
31-
RegExpPrototypeSymbolReplace,
3231
SafeMap,
33-
SafePromiseAll,
32+
SafePromiseAllReturnArrayLike,
33+
SafePromiseAllReturnVoid,
3434
String,
3535
StringFromCharCode,
3636
StringPrototypeEndsWith,
3737
StringPrototypeIncludes,
3838
StringPrototypeRepeat,
39+
StringPrototypeReplaceAll,
3940
StringPrototypeSlice,
4041
StringPrototypeSplit,
4142
StringPrototypeStartsWith,
@@ -53,7 +54,7 @@ const Repl = require('repl');
5354
const vm = require('vm');
5455
const { fileURLToPath } = require('internal/url');
5556

56-
const { customInspectSymbol } = require('internal/util');
57+
const { customInspectSymbol, SideEffectFreeRegExpPrototypeSymbolReplace } = require('internal/util');
5758
const { inspect: utilInspect } = require('internal/util/inspect');
5859
const debuglog = require('internal/util/debuglog').debuglog('inspect');
5960

@@ -121,7 +122,7 @@ const {
121122
} = internalBinding('builtins');
122123
const NATIVES = internalBinding('natives');
123124
function isNativeUrl(url) {
124-
url = RegExpPrototypeSymbolReplace(/\.js$/, url, '');
125+
url = SideEffectFreeRegExpPrototypeSymbolReplace(/\.js$/, url, '');
125126

126127
return StringPrototypeStartsWith(url, 'node:internal/') ||
127128
ArrayPrototypeIncludes(PUBLIC_BUILTINS, url) ||
@@ -159,8 +160,8 @@ function markSourceColumn(sourceText, position, useColors) {
159160

160161
// Colourize char if stdout supports colours
161162
if (useColors) {
162-
tail = RegExpPrototypeSymbolReplace(/(.+?)([^\w]|$)/, tail,
163-
'\u001b[32m$1\u001b[39m$2');
163+
tail = SideEffectFreeRegExpPrototypeSymbolReplace(/(.+?)([^\w]|$)/, tail,
164+
'\u001b[32m$1\u001b[39m$2');
164165
}
165166

166167
// Return source line with coloured char at `position`
@@ -340,9 +341,9 @@ class ScopeSnapshot {
340341
StringPrototypeSlice(this.type, 1);
341342
const name = this.name ? `<${this.name}>` : '';
342343
const prefix = `${type}${name} `;
343-
return RegExpPrototypeSymbolReplace(/^Map /,
344-
utilInspect(this.properties, opts),
345-
prefix);
344+
return SideEffectFreeRegExpPrototypeSymbolReplace(/^Map /,
345+
utilInspect(this.properties, opts),
346+
prefix);
346347
}
347348
}
348349

@@ -517,7 +518,7 @@ function createRepl(inspector) {
517518
}
518519

519520
loadScopes() {
520-
return SafePromiseAll(
521+
return SafePromiseAllReturnArrayLike(
521522
ArrayPrototypeFilter(
522523
this.scopeChain,
523524
(scope) => scope.type !== 'global'
@@ -656,14 +657,14 @@ function createRepl(inspector) {
656657
(error) => `<${error.message}>`);
657658
const lastIndex = watchedExpressions.length - 1;
658659

659-
const values = await SafePromiseAll(watchedExpressions, inspectValue);
660+
const values = await SafePromiseAllReturnArrayLike(watchedExpressions, inspectValue);
660661
const lines = ArrayPrototypeMap(watchedExpressions, (expr, idx) => {
661662
const prefix = `${leftPad(idx, ' ', lastIndex)}: ${expr} =`;
662663
const value = inspect(values[idx]);
663664
if (!StringPrototypeIncludes(value, '\n')) {
664665
return `${prefix} ${value}`;
665666
}
666-
return `${prefix}\n ${RegExpPrototypeSymbolReplace(/\n/g, value, '\n ')}`;
667+
return `${prefix}\n ${StringPrototypeReplaceAll(value, '\n', '\n ')}`;
667668
});
668669
const valueList = ArrayPrototypeJoin(lines, '\n');
669670
return verbose ? `Watchers:\n${valueList}\n` : valueList;
@@ -805,8 +806,8 @@ function createRepl(inspector) {
805806
registerBreakpoint);
806807
}
807808

808-
const escapedPath = RegExpPrototypeSymbolReplace(/([/\\.?*()^${}|[\]])/g,
809-
script, '\\$1');
809+
const escapedPath = SideEffectFreeRegExpPrototypeSymbolReplace(/([/\\.?*()^${}|[\]])/g,
810+
script, '\\$1');
810811
const urlRegex = `^(.*[\\/\\\\])?${escapedPath}$`;
811812

812813
return PromisePrototypeThen(
@@ -860,9 +861,9 @@ function createRepl(inspector) {
860861
location.lineNumber + 1));
861862
if (!newBreakpoints.length) return PromiseResolve();
862863
return PromisePrototypeThen(
863-
SafePromiseAll(newBreakpoints),
864-
(results) => {
865-
print(`${results.length} breakpoints restored.`);
864+
SafePromiseAllReturnVoid(newBreakpoints),
865+
() => {
866+
print(`${newBreakpoints.length} breakpoints restored.`);
866867
});
867868
}
868869

@@ -896,7 +897,7 @@ function createRepl(inspector) {
896897

897898
inspector.suspendReplWhile(() =>
898899
PromisePrototypeThen(
899-
SafePromiseAll([formatWatchers(true), selectedFrame.list(2)]),
900+
SafePromiseAllReturnArrayLike([formatWatchers(true), selectedFrame.list(2)]),
900901
({ 0: watcherList, 1: context }) => {
901902
const breakContext = watcherList ?
902903
`${watcherList}\n${inspect(context)}` :
Collapse file

‎lib/internal/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/util.js
+42Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,24 @@ const {
2323
ReflectApply,
2424
ReflectConstruct,
2525
RegExpPrototypeExec,
26+
RegExpPrototypeGetDotAll,
27+
RegExpPrototypeGetGlobal,
28+
RegExpPrototypeGetHasIndices,
29+
RegExpPrototypeGetIgnoreCase,
30+
RegExpPrototypeGetMultiline,
31+
RegExpPrototypeGetSticky,
32+
RegExpPrototypeGetUnicode,
33+
RegExpPrototypeGetSource,
2634
SafeMap,
2735
SafeSet,
36+
SafeWeakMap,
2837
StringPrototypeReplace,
2938
StringPrototypeToLowerCase,
3039
StringPrototypeToUpperCase,
3140
Symbol,
3241
SymbolFor,
42+
SymbolReplace,
43+
SymbolSplit,
3344
} = primordials;
3445

3546
const {
@@ -559,6 +570,35 @@ function SideEffectFreeRegExpPrototypeExec(regex, string) {
559570
return FunctionPrototypeCall(RegExpFromAnotherRealm.prototype.exec, regex, string);
560571
}
561572

573+
const crossRelmRegexes = new SafeWeakMap();
574+
function getCrossRelmRegex(regex) {
575+
const cached = crossRelmRegexes.get(regex);
576+
if (cached) return cached;
577+
578+
let flagString = '';
579+
if (RegExpPrototypeGetHasIndices(regex)) flagString += 'd';
580+
if (RegExpPrototypeGetGlobal(regex)) flagString += 'g';
581+
if (RegExpPrototypeGetIgnoreCase(regex)) flagString += 'i';
582+
if (RegExpPrototypeGetMultiline(regex)) flagString += 'm';
583+
if (RegExpPrototypeGetDotAll(regex)) flagString += 's';
584+
if (RegExpPrototypeGetUnicode(regex)) flagString += 'u';
585+
if (RegExpPrototypeGetSticky(regex)) flagString += 'y';
586+
587+
const { RegExp: RegExpFromAnotherRealm } = getInternalGlobal();
588+
const crossRelmRegex = new RegExpFromAnotherRealm(RegExpPrototypeGetSource(regex), flagString);
589+
crossRelmRegexes.set(regex, crossRelmRegex);
590+
return crossRelmRegex;
591+
}
592+
593+
function SideEffectFreeRegExpPrototypeSymbolReplace(regex, string, replacement) {
594+
return getCrossRelmRegex(regex)[SymbolReplace](string, replacement);
595+
}
596+
597+
function SideEffectFreeRegExpPrototypeSymbolSplit(regex, string, limit = undefined) {
598+
return getCrossRelmRegex(regex)[SymbolSplit](string, limit);
599+
}
600+
601+
562602
function isArrayBufferDetached(value) {
563603
if (ArrayBufferPrototypeGetByteLength(value) === 0) {
564604
return _isArrayBufferDetached(value);
@@ -594,6 +634,8 @@ module.exports = {
594634
once,
595635
promisify,
596636
SideEffectFreeRegExpPrototypeExec,
637+
SideEffectFreeRegExpPrototypeSymbolReplace,
638+
SideEffectFreeRegExpPrototypeSymbolSplit,
597639
sleep,
598640
spliceOne,
599641
toUSVString,
Collapse file

‎lib/repl.js‎

Copy file name to clipboardExpand all lines: lib/repl.js
+9-9Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ const {
7777
ReflectApply,
7878
RegExp,
7979
RegExpPrototypeExec,
80-
RegExpPrototypeSymbolReplace,
81-
RegExpPrototypeSymbolSplit,
8280
SafePromiseRace,
8381
SafeSet,
8482
SafeWeakSet,
@@ -111,7 +109,9 @@ const {
111109
const {
112110
decorateErrorStack,
113111
isError,
114-
deprecate
112+
deprecate,
113+
SideEffectFreeRegExpPrototypeSymbolReplace,
114+
SideEffectFreeRegExpPrototypeSymbolSplit,
115115
} = require('internal/util');
116116
const { inspect } = require('internal/util/inspect');
117117
const vm = require('vm');
@@ -456,7 +456,7 @@ function REPLServer(prompt,
456456

457457
// Remove all "await"s and attempt running the script
458458
// in order to detect if error is truly non recoverable
459-
const fallbackCode = RegExpPrototypeSymbolReplace(/\bawait\b/g, code, '');
459+
const fallbackCode = SideEffectFreeRegExpPrototypeSymbolReplace(/\bawait\b/g, code, '');
460460
try {
461461
vm.createScript(fallbackCode, {
462462
filename: file,
@@ -681,22 +681,22 @@ function REPLServer(prompt,
681681
if (e.stack) {
682682
if (e.name === 'SyntaxError') {
683683
// Remove stack trace.
684-
e.stack = RegExpPrototypeSymbolReplace(
684+
e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
685685
/^\s+at\s.*\n?/gm,
686-
RegExpPrototypeSymbolReplace(/^REPL\d+:\d+\r?\n/, e.stack, ''),
686+
SideEffectFreeRegExpPrototypeSymbolReplace(/^REPL\d+:\d+\r?\n/, e.stack, ''),
687687
'');
688688
const importErrorStr = 'Cannot use import statement outside a ' +
689689
'module';
690690
if (StringPrototypeIncludes(e.message, importErrorStr)) {
691691
e.message = 'Cannot use import statement inside the Node.js ' +
692692
'REPL, alternatively use dynamic import';
693-
e.stack = RegExpPrototypeSymbolReplace(
693+
e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
694694
/SyntaxError:.*\n/,
695695
e.stack,
696696
`SyntaxError: ${e.message}\n`);
697697
}
698698
} else if (self.replMode === module.exports.REPL_MODE_STRICT) {
699-
e.stack = RegExpPrototypeSymbolReplace(
699+
e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
700700
/(\s+at\s+REPL\d+:)(\d+)/,
701701
e.stack,
702702
(_, pre, line) => pre + (line - 1)
@@ -728,7 +728,7 @@ function REPLServer(prompt,
728728
if (errStack === '') {
729729
errStack = self.writer(e);
730730
}
731-
const lines = RegExpPrototypeSymbolSplit(/(?<=\n)/, errStack);
731+
const lines = SideEffectFreeRegExpPrototypeSymbolSplit(/(?<=\n)/, errStack);
732732
let matched = false;
733733

734734
errStack = '';
Collapse file

‎test/parallel/test-primordials-regexp.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-primordials-regexp.js
+55Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,20 @@ const { mustNotCall } = require('../common');
55
const assert = require('assert');
66

77
const {
8+
RegExpPrototypeExec,
89
RegExpPrototypeSymbolReplace,
910
RegExpPrototypeSymbolSearch,
1011
RegExpPrototypeSymbolSplit,
1112
SafeStringPrototypeSearch,
1213
hardenRegExp,
1314
} = require('internal/test/binding').primordials;
1415

16+
const {
17+
SideEffectFreeRegExpPrototypeExec,
18+
SideEffectFreeRegExpPrototypeSymbolReplace,
19+
SideEffectFreeRegExpPrototypeSymbolSplit,
20+
} = require('internal/util');
21+
1522

1623
Object.defineProperties(RegExp.prototype, {
1724
[Symbol.match]: {
@@ -89,14 +96,46 @@ hardenRegExp(hardenRegExp(/1/));
8996
// IMO there are no valid use cases in node core to use RegExpPrototypeSymbolMatch
9097
// or RegExpPrototypeSymbolMatchAll, they are inherently unsafe.
9198

99+
assert.strictEqual(RegExpPrototypeExec(/foo/, 'bar'), null);
100+
assert.strictEqual(RegExpPrototypeExec(hardenRegExp(/foo/), 'bar'), null);
101+
assert.strictEqual(SideEffectFreeRegExpPrototypeExec(/foo/, 'bar'), null);
102+
assert.strictEqual(SideEffectFreeRegExpPrototypeExec(hardenRegExp(/foo/), 'bar'), null);
103+
{
104+
const expected = ['bar'];
105+
Object.defineProperties(expected, {
106+
index: { __proto__: null, configurable: true, writable: true, enumerable: true, value: 0 },
107+
input: { __proto__: null, configurable: true, writable: true, enumerable: true, value: 'bar' },
108+
groups: { __proto__: null, configurable: true, writable: true, enumerable: true },
109+
});
110+
const actual = SideEffectFreeRegExpPrototypeExec(/bar/, 'bar');
111+
112+
// assert.deepStrictEqual(actual, expected) doesn't work for cross-realm comparison.
113+
114+
assert.strictEqual(Array.isArray(actual), Array.isArray(expected));
115+
assert.deepStrictEqual(Reflect.ownKeys(actual), Reflect.ownKeys(expected));
116+
for (const key of Reflect.ownKeys(expected)) {
117+
assert.deepStrictEqual(
118+
Reflect.getOwnPropertyDescriptor(actual, key),
119+
Reflect.getOwnPropertyDescriptor(expected, key),
120+
);
121+
}
122+
}
92123
{
93124
const myRegex = hardenRegExp(/a/);
94125
assert.strictEqual(RegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'bear');
95126
}
127+
{
128+
const myRegex = /a/;
129+
assert.strictEqual(SideEffectFreeRegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'bear');
130+
}
96131
{
97132
const myRegex = hardenRegExp(/a/g);
98133
assert.strictEqual(RegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'beer');
99134
}
135+
{
136+
const myRegex = /a/g;
137+
assert.strictEqual(SideEffectFreeRegExpPrototypeSymbolReplace(myRegex, 'baar', 'e'), 'beer');
138+
}
100139
{
101140
const myRegex = hardenRegExp(/a/);
102141
assert.strictEqual(RegExpPrototypeSymbolSearch(myRegex, 'baar'), 1);
@@ -109,6 +148,22 @@ hardenRegExp(hardenRegExp(/1/));
109148
const myRegex = hardenRegExp(/a/);
110149
assert.deepStrictEqual(RegExpPrototypeSymbolSplit(myRegex, 'baar', 0), []);
111150
}
151+
{
152+
const myRegex = /a/;
153+
const expected = [];
154+
const actual = SideEffectFreeRegExpPrototypeSymbolSplit(myRegex, 'baar', 0);
155+
156+
// assert.deepStrictEqual(actual, expected) doesn't work for cross-realm comparison.
157+
158+
assert.strictEqual(Array.isArray(actual), Array.isArray(expected));
159+
assert.deepStrictEqual(Reflect.ownKeys(actual), Reflect.ownKeys(expected));
160+
for (const key of Reflect.ownKeys(expected)) {
161+
assert.deepStrictEqual(
162+
Reflect.getOwnPropertyDescriptor(actual, key),
163+
Reflect.getOwnPropertyDescriptor(expected, key),
164+
);
165+
}
166+
}
112167
{
113168
const myRegex = hardenRegExp(/a/);
114169
assert.deepStrictEqual(RegExpPrototypeSymbolSplit(myRegex, 'baar', 1), ['b']);
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-startup-empty-regexp-statics.js
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,23 @@ const allRegExpStatics =
4040
assert.strictEqual(child.signal, null);
4141
}
4242

43+
{
44+
const child = spawnSync(process.execPath,
45+
[ '--expose-internals', '-p', `const {
46+
SideEffectFreeRegExpPrototypeExec,
47+
SideEffectFreeRegExpPrototypeSymbolReplace,
48+
SideEffectFreeRegExpPrototypeSymbolSplit,
49+
} = require("internal/util");
50+
SideEffectFreeRegExpPrototypeExec(/foo/, "foo");
51+
SideEffectFreeRegExpPrototypeSymbolReplace(/o/, "foo", "a");
52+
SideEffectFreeRegExpPrototypeSymbolSplit(/o/, "foo");
53+
${allRegExpStatics}` ],
54+
{ stdio: ['inherit', 'pipe', 'inherit'] });
55+
assert.match(child.stdout.toString(), /^undefined\r?\n$/);
56+
assert.strictEqual(child.status, 0);
57+
assert.strictEqual(child.signal, null);
58+
}
59+
4360
{
4461
const child = spawnSync(process.execPath,
4562
[ '-e', `console.log(${allRegExpStatics})`, '--input-type=module' ],

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.