Skip to content

Commit

Permalink
repl: no RegExp side effects for static properties
Browse files Browse the repository at this point in the history
Disallow global RegExp object usage inside repl.js,
readline.js and tty.js. Use RegExp from internal context
to prevent updating global.RegExp static properties

tools/eslint-rules/no-regex-literal-for-repl.js linter is added
to prevent regex literal usage in repl code path

Fixes: nodejs#18931
  • Loading branch information
princejwesley committed May 5, 2018
1 parent d7cba76 commit ea9878e
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 67 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ module.exports = {

// Custom rules from eslint-plugin-node-core
'node-core/no-unescaped-regexp-dot': 'error',
'node-core/no-regex-literal-for-repl': 'error',
},
globals: {
COUNTER_HTTP_CLIENT_REQUEST: false,
Expand Down
24 changes: 12 additions & 12 deletions lib/internal/readline.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
'use strict';

// Regex used for ansi escape code splitting
// Adopted from https://github.com/chalk/ansi-regex/blob/master/index.js
// License: MIT, authors: @sindresorhus, Qix-, and arjunmehta
// Matches all ansi escape code sequences in a string
/* eslint-disable no-control-regex */
const ansi =
/[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g;
/* eslint-enable no-control-regex */
const { getInternalGlobal } = require('internal/util');
const { RegExp } = getInternalGlobal();

const ansiPattern =
'[\\u001b\\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?' +
'[0-9A-ORZcf-nqry=><]';
const ansi = new RegExp(ansiPattern, 'g');

const kEscape = '\x1b';

Expand Down Expand Up @@ -267,10 +266,11 @@ function* emitKeys(stream) {
const cmd = s.slice(cmdStart);
let match;

if ((match = cmd.match(/^(\d\d?)(;(\d))?([~^$])$/))) {
if ((match = cmd.match(new RegExp('^(\\d\\d?)(;(\\d))?([~^$])$')))) {
code += match[1] + match[4];
modifier = (match[3] || 1) - 1;
} else if ((match = cmd.match(/^((\d;)?(\d))?([A-Za-z])$/))) {
} else if ((match =
cmd.match(new RegExp('^((\\d;)?(\\d))?([A-Za-z])$')))) {
code += match[4];
modifier = (match[3] || 1) - 1;
} else {
Expand Down Expand Up @@ -404,10 +404,10 @@ function* emitKeys(stream) {
// ctrl+letter
key.name = String.fromCharCode(ch.charCodeAt(0) + 'a'.charCodeAt(0) - 1);
key.ctrl = true;
} else if (/^[0-9A-Za-z]$/.test(ch)) {
} else if (new RegExp('^[0-9A-Za-z]$').test(ch)) {
// letter, number, shift+letter
key.name = ch.toLowerCase();
key.shift = /^[A-Z]$/.test(ch);
key.shift = new RegExp('^[A-Z]$').test(ch);
key.meta = escaped;
} else if (escaped) {
// Escape sequence timeout
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ const path = require('path');
const fs = require('fs');
const os = require('os');
const util = require('util');
const { getInternalGlobal } = require('internal/util');
const { RegExp } = getInternalGlobal();

const debug = util.debuglog('repl');
module.exports = Object.create(REPL);
module.exports.createInternalRepl = createRepl;
Expand Down Expand Up @@ -128,7 +131,7 @@ function setupHistory(repl, historyPath, ready) {
}

if (data) {
repl.history = data.split(/[\n\r]+/, repl.historySize);
repl.history = data.split(new RegExp('[\\n\\r]+'), repl.historySize);
} else {
repl.history = [];
}
Expand Down
25 changes: 14 additions & 11 deletions lib/internal/tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

'use strict';

const { getInternalGlobal } = require('internal/util');
const { RegExp } = getInternalGlobal();
const { release } = require('os');

const OSRelease = release().split('.');
Expand Down Expand Up @@ -56,14 +58,14 @@ const TERM_ENVS = [
];

const TERM_ENVS_REG_EXP = [
/ansi/,
/color/,
/linux/,
/^con[0-9]*x[0-9]/,
/^rxvt/,
/^screen/,
/^xterm/,
/^vt100/
new RegExp('ansi'),
new RegExp('color'),
new RegExp('linux'),
new RegExp('^con[0-9]*x[0-9]'),
new RegExp('^rxvt'),
new RegExp('^screen'),
new RegExp('^xterm'),
new RegExp('^vt100')
];

// The `getColorDepth` API got inspired by multiple sources such as
Expand Down Expand Up @@ -102,14 +104,15 @@ function getColorDepth(env = process.env) {
}

if ('TEAMCITY_VERSION' in env) {
return /^(9\.(0*[1-9]\d*)\.|\d{2,}\.)/.test(env.TEAMCITY_VERSION) ?
return new RegExp('^(9\\.(0*[1-9]\\d*)\\.|\\d{2,}\\.)')
.test(env.TEAMCITY_VERSION) ?
COLORS_16 : COLORS_2;
}

switch (env.TERM_PROGRAM) {
case 'iTerm.app':
if (!env.TERM_PROGRAM_VERSION ||
/^[0-2]\./.test(env.TERM_PROGRAM_VERSION)) {
new RegExp('^[0-2]\\.').test(env.TERM_PROGRAM_VERSION)) {
return COLORS_256;
}
return COLORS_16m;
Expand All @@ -122,7 +125,7 @@ function getColorDepth(env = process.env) {
}

if (env.TERM) {
if (/^xterm-256/.test(env.TERM))
if (new RegExp('^xterm-256').test(env.TERM))
return COLORS_256;

const termEnv = env.TERM.toLowerCase();
Expand Down
12 changes: 12 additions & 0 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,17 @@ function isInsideNodeModules() {
return false;
}

let internalGlobal;

function getInternalGlobal() {
if (internalGlobal === undefined) {
const { runInNewContext } = require('vm');
internalGlobal = runInNewContext('this');
}

return internalGlobal;
}


module.exports = {
assertCrypto,
Expand All @@ -390,6 +401,7 @@ module.exports = {
promisify,
spliceOne,
removeColors,
getInternalGlobal,

// Symbol used to customize promisify conversion
customPromisifyArgs: kCustomPromisifyArgsSymbol,
Expand Down
17 changes: 10 additions & 7 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const {
stripVTControlCharacters
} = require('internal/readline');

const { getInternalGlobal } = require('internal/util');
const { RegExp } = getInternalGlobal();

const {
kEscape,
kClearToBeginning,
Expand All @@ -55,7 +58,7 @@ const {
const kHistorySize = 30;
const kMincrlfDelay = 100;
// \r\n, \n, or \r followed by something other than \n
const lineEnding = /\r?\n|\r(?!\n)/;
const lineEnding = new RegExp('\\r?\\n|\\r(?!\\n)');

const KEYPRESS_DECODER = Symbol('keypress-decoder');
const ESCAPE_DECODER = Symbol('escape-decoder');
Expand Down Expand Up @@ -405,7 +408,7 @@ Interface.prototype._normalWrite = function(b) {
var string = this._decoder.write(b);
if (this._sawReturnAt &&
Date.now() - this._sawReturnAt <= this.crlfDelay) {
string = string.replace(/^\n/, '');
string = string.replace(new RegExp('^\\n'), '');
this._sawReturnAt = 0;
}

Expand Down Expand Up @@ -549,7 +552,7 @@ function commonPrefix(strings) {
Interface.prototype._wordLeft = function() {
if (this.cursor > 0) {
var leading = this.line.slice(0, this.cursor);
var match = leading.match(/(?:[^\w\s]+|\w+|)\s*$/);
var match = leading.match(new RegExp('(?:[^\\w\\s]+|\\w+|)\\s*$'));
this._moveCursor(-match[0].length);
}
};
Expand All @@ -558,7 +561,7 @@ Interface.prototype._wordLeft = function() {
Interface.prototype._wordRight = function() {
if (this.cursor < this.line.length) {
var trailing = this.line.slice(this.cursor);
var match = trailing.match(/^(?:\s+|\W+|\w+)\s*/);
var match = trailing.match(new RegExp('^(?:\\s+|\\W+|\\w+)\\s*'));
this._moveCursor(match[0].length);
}
};
Expand All @@ -585,7 +588,7 @@ Interface.prototype._deleteRight = function() {
Interface.prototype._deleteWordLeft = function() {
if (this.cursor > 0) {
var leading = this.line.slice(0, this.cursor);
var match = leading.match(/(?:[^\w\s]+|\w+|)\s*$/);
var match = leading.match(new RegExp('(?:[^\\w\\s]+|\\w+|)\\s*$'));
leading = leading.slice(0, leading.length - match[0].length);
this.line = leading + this.line.slice(this.cursor, this.line.length);
this.cursor = leading.length;
Expand All @@ -597,7 +600,7 @@ Interface.prototype._deleteWordLeft = function() {
Interface.prototype._deleteWordRight = function() {
if (this.cursor < this.line.length) {
var trailing = this.line.slice(this.cursor);
var match = trailing.match(/^(?:\s+|\W+|\w+)\s*/);
var match = trailing.match(new RegExp('^(?:\\s+|\\W+|\\w+)\\s*'));
this.line = this.line.slice(0, this.cursor) +
trailing.slice(match[0].length);
this._refreshLine();
Expand Down Expand Up @@ -969,7 +972,7 @@ Interface.prototype._ttyWrite = function(s, key) {
s = s.toString('utf-8');

if (s) {
var lines = s.split(/\r\n|\n|\r/);
var lines = s.split(new RegExp('\\r\\n|\\n|\\r'));
for (var i = 0, len = lines.length; i < len; i++) {
if (i > 0) {
this._line();
Expand Down
Loading

0 comments on commit ea9878e

Please sign in to comment.