From ea9878e0d88e1e892d985851970a54bad92c51ec Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Sun, 6 May 2018 00:33:36 +0530 Subject: [PATCH] repl: no RegExp side effects for static properties 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: #18931 --- .eslintrc.js | 1 + lib/internal/readline.js | 24 ++++---- lib/internal/repl.js | 5 +- lib/internal/tty.js | 25 ++++---- lib/internal/util.js | 12 ++++ lib/readline.js | 17 +++--- lib/repl.js | 60 ++++++++---------- .../eslint-rules/no-regex-literal-for-repl.js | 61 +++++++++++++++++++ 8 files changed, 138 insertions(+), 67 deletions(-) create mode 100644 tools/eslint-rules/no-regex-literal-for-repl.js diff --git a/.eslintrc.js b/.eslintrc.js index 53d0b39d0aea44..27652221f1e0d3 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -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, diff --git a/lib/internal/readline.js b/lib/internal/readline.js index 5d7cbd32f362b3..245403a693c618 100644 --- a/lib/internal/readline.js +++ b/lib/internal/readline.js @@ -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'; @@ -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 { @@ -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 diff --git a/lib/internal/repl.js b/lib/internal/repl.js index 4c4e3c8cb4e275..189609f07e0488 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -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; @@ -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 = []; } diff --git a/lib/internal/tty.js b/lib/internal/tty.js index c4edab24fb87ab..0831be04ad18e2 100644 --- a/lib/internal/tty.js +++ b/lib/internal/tty.js @@ -22,6 +22,8 @@ 'use strict'; +const { getInternalGlobal } = require('internal/util'); +const { RegExp } = getInternalGlobal(); const { release } = require('os'); const OSRelease = release().split('.'); @@ -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 @@ -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; @@ -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(); diff --git a/lib/internal/util.js b/lib/internal/util.js index 07515e2e090daa..54faaadd95b54c 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -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, @@ -390,6 +401,7 @@ module.exports = { promisify, spliceOne, removeColors, + getInternalGlobal, // Symbol used to customize promisify conversion customPromisifyArgs: kCustomPromisifyArgsSymbol, diff --git a/lib/readline.js b/lib/readline.js index 124fc8111b2f09..778f5e080f01cd 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -44,6 +44,9 @@ const { stripVTControlCharacters } = require('internal/readline'); +const { getInternalGlobal } = require('internal/util'); +const { RegExp } = getInternalGlobal(); + const { kEscape, kClearToBeginning, @@ -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'); @@ -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; } @@ -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); } }; @@ -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); } }; @@ -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; @@ -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(); @@ -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(); diff --git a/lib/repl.js b/lib/repl.js index a7977dc72fb781..69ace9211b6afc 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -69,6 +69,7 @@ const { } = require('internal/errors').codes; const { sendInspectorCommand } = require('internal/util/inspector'); const { experimentalREPLAwait } = process.binding('config'); +const { RegExp } = internalUtil.getInternalGlobal(); // Lazy-loaded. let processTopLevelAwait; @@ -178,12 +179,6 @@ function REPLServer(prompt, // Just for backwards compat, see github.com/joyent/node/pull/7127 self.rli = this; - const savedRegExMatches = ['', '', '', '', '', '', '', '', '', '']; - const sep = '\u0000\u0000\u0000'; - const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` + - `${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` + - `${sep}(.*)$`); - eval_ = eval_ || defaultEval; // Pause taking in new input, and store the keys in a buffer. @@ -220,7 +215,8 @@ function REPLServer(prompt, var awaitPromise = false; var input = code; - if (/^\s*\{/.test(code) && /\}\s*$/.test(code)) { + if (new RegExp('^\\s*\\{').test(code) && + new RegExp('\\}\\s*$').test(code)) { // It's confusing for `{ a : 1 }` to be interpreted as a block // statement rather than an object literal. So, we first try // to wrap it in parentheses, so that it will be interpreted as @@ -248,7 +244,7 @@ function REPLServer(prompt, while (true) { try { - if (!/^\s*$/.test(code) && + if (!new RegExp('^\\s*$').test(code) && self.replMode === exports.REPL_MODE_STRICT) { // "void 0" keeps the repl from returning "use strict" as the result // value for statements and declarations that don't return a value. @@ -278,21 +274,11 @@ function REPLServer(prompt, break; } - // This will set the values from `savedRegExMatches` to corresponding - // predefined RegExp properties `RegExp.$1`, `RegExp.$2` ... `RegExp.$9` - regExMatcher.test(savedRegExMatches.join(sep)); - let finished = false; function finishExecution(err, result) { if (finished) return; finished = true; - // After executing the current expression, store the values of RegExp - // predefined properties back in `savedRegExMatches` - for (var idx = 1; idx < savedRegExMatches.length; idx += 1) { - savedRegExMatches[idx] = RegExp[`$${idx}`]; - } - cb(err, result); } @@ -411,10 +397,10 @@ function REPLServer(prompt, if (e instanceof SyntaxError && e.stack) { // remove repl:line-number and stack trace e.stack = e.stack - .replace(/^repl:\d+\r?\n/, '') - .replace(/^\s+at\s.*\n?/gm, ''); + .replace(new RegExp('^repl:\\d+\\r?\\n'), '') + .replace(new RegExp('^\\s+at\\s.*\\n?', 'gm'), ''); } else if (isError && self.replMode === exports.REPL_MODE_STRICT) { - e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/, + e.stack = e.stack.replace(new RegExp('(\\s+at\\s+repl:)(\\d+)'), (_, pre, line) => pre + (line - 1)); } if (isError && e.stack) { @@ -577,7 +563,7 @@ function REPLServer(prompt, self[kBufferedCommandSymbol] += cmd + '\n'; // code alignment - const matches = self._sawKeyPress ? cmd.match(/^\s+/) : null; + const matches = self._sawKeyPress ? cmd.match(new RegExp('^\\s+')) : null; if (matches) { const prefix = matches[0]; self.write(prefix); @@ -596,7 +582,7 @@ function REPLServer(prompt, if (trimmedCmd) { if (trimmedCmd.charAt(0) === '.' && trimmedCmd.charAt(1) !== '.' && Number.isNaN(parseFloat(trimmedCmd))) { - const matches = trimmedCmd.match(/^\.([^\s]+)\s*(.*)$/); + const matches = trimmedCmd.match(new RegExp('^\\.([^\\s]+)\\s*(.*)$')); const keyword = matches && matches[1]; const rest = matches && matches[2]; if (_parseREPLKeyword.call(self, keyword, rest) === true) { @@ -890,13 +876,14 @@ ArrayStream.prototype.writable = true; ArrayStream.prototype.resume = function() {}; ArrayStream.prototype.write = function() {}; -const requireRE = /\brequire\s*\(['"](([\w@./-]+\/)?(?:[\w@./-]*))/; +const requireRE = + new RegExp('\\brequire\\s*\\([\'"](([\\w@.\\/-]+\\/)?(?:[\\w@.\\/-]*))'); const simpleExpressionRE = - /(?:[a-zA-Z_$](?:\w|\$)*\.)*[a-zA-Z_$](?:\w|\$)*\.?$/; + new RegExp('(?:[a-zA-Z_$](?:\\w|\\$)*\\.)*[a-zA-Z_$](?:\\w|\\$)*\\.?$'); function intFilter(item) { // filters out anything not starting with A-Z, a-z, $ or _ - return /^[A-Za-z_$]/.test(item); + return new RegExp('^[A-Za-z_$]').test(item); } const ARRAY_LENGTH_THRESHOLD = 1e6; @@ -987,7 +974,7 @@ function complete(line, callback) { // REPL commands (e.g. ".break"). var filter; var match = null; - match = line.match(/^\s*\.(\w*)$/); + match = line.match(new RegExp('^\\s*\\.(\\w*)$')); if (match) { completionGroups.push(Object.keys(this.commands)); completeOn = match[1]; @@ -1001,7 +988,7 @@ function complete(line, callback) { const exts = Object.keys(this.context.require.extensions); var indexRe = new RegExp('^index(?:' + exts.map(regexpEscape).join('|') + ')$'); - var versionedFileNamesRe = /-\d+\.\d+/; + var versionedFileNamesRe = new RegExp('-\\d+\\.\\d+'); completeOn = match[1]; var subdir = match[2] || ''; @@ -1014,7 +1001,7 @@ function complete(line, callback) { group = ['./', '../']; } else if (completeOn === '..') { group = ['../']; - } else if (/^\.\.?\//.test(completeOn)) { + } else if (new RegExp('^\\.\\.?\\/').test(completeOn)) { paths = [process.cwd()]; } else { paths = module.paths.concat(CJSModule.globalPaths); @@ -1078,7 +1065,8 @@ function complete(line, callback) { // spam.eggs.<|> # completions for 'spam.eggs' with filter '' // foo<|> # all scope vars with filter 'foo' // foo.<|> # completions for 'foo' with filter '' - } else if (line.length === 0 || /\w|\.|\$/.test(line[line.length - 1])) { + } else if (line.length === 0 || + new RegExp('\\w|\\.|\\$').test(line[line.length - 1])) { match = simpleExpressionRE.exec(line); if (line.length === 0 || match) { var expr; @@ -1293,8 +1281,8 @@ function _memory(cmd) { if (cmd) { // Going down is { and ( e.g. function() { // going up is } and ) - var dw = cmd.match(/{|\(/g); - var up = cmd.match(/}|\)/g); + var dw = cmd.match(new RegExp('{|\\(', 'g')); + var up = cmd.match(new RegExp('}|\\)', 'g')); up = up ? up.length : 0; dw = dw ? dw.length : 0; var depth = dw - up; @@ -1312,7 +1300,7 @@ function _memory(cmd) { self.lines.level.push({ line: self.lines.length - 1, depth: depth, - isFunction: /\bfunction\b/.test(cmd) + isFunction: new RegExp('\\bfunction\\b').test(cmd) }); } else if (depth < 0) { // Going... up. @@ -1476,7 +1464,7 @@ function defineDefaultCommands(repl) { } function regexpEscape(s) { - return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&'); + return s.replace(new RegExp('[-[\\]{}()*+?.,\\\\^$|#\\s]', 'g'), '\\$&'); } // If the error is that we've unexpectedly ended the input, @@ -1490,8 +1478,8 @@ function isRecoverableError(e, code) { } if (message === 'missing ) after argument list') { - const frames = e.stack.split(/\r?\n/); - const pos = frames.findIndex((f) => f.match(/^\s*\^+$/)); + const frames = e.stack.split(new RegExp('\\r?\\n')); + const pos = frames.findIndex((f) => f.match(new RegExp('^\\s*\\^+$'))); return pos > 0 && frames[pos - 1].length === frames[pos].length; } diff --git a/tools/eslint-rules/no-regex-literal-for-repl.js b/tools/eslint-rules/no-regex-literal-for-repl.js new file mode 100644 index 00000000000000..15039ad62f8001 --- /dev/null +++ b/tools/eslint-rules/no-regex-literal-for-repl.js @@ -0,0 +1,61 @@ +/** + * @fileoverview Look for regex literal in set of repl and its dependency files + * @author Prince J Wesley + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +// Exclude file list +// Make sure that all exclude files has RegExp instance +// from require('internal/util').getInternalGlobal() + +// this linter rule prevents accidental access to global regexp literal + +const fileNames = [ + 'lib/internal/repl.js', + 'lib/internal/readline.js', + 'lib/internal/tty.js', + 'lib/repl.js', + 'lib/readline.js', + 'lib/tty.js' +]; + +const isExcludedFile = (name) => + !!fileNames.find((n) => name.indexOf(n) !== -1); + +module.exports = function(context) { + const sourceCode = context.getSourceCode(); + const filename = context.getFilename(); + const excluded = isExcludedFile(filename); + + function report(node) { + context.report({ + node, + message: `Global RegExp literal is not allowed in ${filename}`, + fix(fixer) { + const regex = node.regex; + const pattern = regex.pattern.replace(/\\/g, '\\\\'); + let replaceText = `new RegExp('${pattern}'`; + replaceText += regex.flags ? `, '${regex.flags}')` : ')'; + return fixer.replaceText(node, replaceText); + } + }); + } + + function checkLiteral(node) { + if (!excluded) return; + + const { type } = sourceCode.getFirstToken(node); + if (type === 'RegularExpression') { + report(node); + } + + } + + return { + Literal: checkLiteral + }; +};