Skip to content

Commit

Permalink
Update: consider env in no-implied-eval (fixes #12733) (#12757)
Browse files Browse the repository at this point in the history 
* Fix: consider env in no-implied-eval (fixes #12733)

* fix typo, refactor to chaining

* change to check only CallExpression, refactoring

* check defs

* checks callee

* check static string value

* check falsy - first argument

* check empty arg

* fix typo
  • Loading branch information
@yeonjuan
yeonjuan committed Mar 21, 2020
1 parent 9ac5b9e commit 085979f
Show file tree
Hide file tree
Showing 2 changed files with 215 additions and 107 deletions.
184 changes: 83 additions & 101 deletions lib/rules/no-implied-eval.js
View file
Expand Up @@ -5,6 +5,13 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");
const { getStaticValue } = require("eslint-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -28,94 +35,97 @@ module.exports = {
},

create(context) {
const CALLEE_RE = /^(setTimeout|setInterval|execScript)$/u;

/*
* Figures out if we should inspect a given binary expression. Is a stack
* of stacks, where the first element in each substack is a CallExpression.
*/
const impliedEvalAncestorsStack = [];

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------
const EVAL_LIKE_FUNCS = Object.freeze(["setTimeout", "execScript", "setInterval"]);
const GLOBAL_CANDIDATES = Object.freeze(["global", "window"]);

/**
* Get the last element of an array, without modifying arr, like pop(), but non-destructive.
* @param {Array} arr What to inspect
* @returns {*} The last element of arr
* @private
* Checks whether a node is evaluated as a string or not.
* @param {ASTNode} node A node to check.
* @returns {boolean} True if the node is evaluated as a string.
*/
function last(arr) {
return arr ? arr[arr.length - 1] : null;
function isEvaluatedString(node) {
if (
(node.type === "Literal" && typeof node.value === "string") ||
node.type === "TemplateLiteral"
) {
return true;
}
if (node.type === "BinaryExpression" && node.operator === "+") {
return isEvaluatedString(node.left) || isEvaluatedString(node.right);
}
return false;
}

/**
* Checks if the given MemberExpression node is a potentially implied eval identifier on window.
* @param {ASTNode} node The MemberExpression node to check.
* @returns {boolean} Whether or not the given node is potentially an implied eval.
* @private
* Checks whether a node is an Identifier node named one of the specified names.
* @param {ASTNode} node A node to check.
* @param {string[]} specifiers Array of specified name.
* @returns {boolean} True if the node is a Identifier node which has specified name.
*/
function isImpliedEvalMemberExpression(node) {
const object = node.object,
property = node.property,
hasImpliedEvalName = CALLEE_RE.test(property.name) || CALLEE_RE.test(property.value);

return object.name === "window" && hasImpliedEvalName;
function isSpecifiedIdentifier(node, specifiers) {
return node.type === "Identifier" && specifiers.includes(node.name);
}

/**
* Determines if a node represents a call to a potentially implied eval.
*
* This checks the callee name and that there's an argument, but not the type of the argument.
* @param {ASTNode} node The CallExpression to check.
* @returns {boolean} True if the node matches, false if not.
* @private
* Checks a given node is a MemberExpression node which has the specified name's
* property.
* @param {ASTNode} node A node to check.
* @param {string[]} specifiers Array of specified name.
* @returns {boolean} `true` if the node is a MemberExpression node which has
* the specified name's property
*/
function isImpliedEvalCallExpression(node) {
const isMemberExpression = (node.callee.type === "MemberExpression"),
isIdentifier = (node.callee.type === "Identifier"),
isImpliedEvalCallee =
(isIdentifier && CALLEE_RE.test(node.callee.name)) ||
(isMemberExpression && isImpliedEvalMemberExpression(node.callee));

return isImpliedEvalCallee && node.arguments.length;
function isSpecifiedMember(node, specifiers) {
return node.type === "MemberExpression" && specifiers.includes(astUtils.getStaticPropertyName(node));
}

/**
* Checks that the parent is a direct descendent of an potential implied eval CallExpression, and if the parent is a CallExpression, that we're the first argument.
* @param {ASTNode} node The node to inspect the parent of.
* @returns {boolean} Was the parent a direct descendent, and is the child therefore potentially part of a dangerous argument?
* @private
* Reports if the `CallExpression` node has evaluated argument.
* @param {ASTNode} node A CallExpression to check.
* @returns {void}
*/
function hasImpliedEvalParent(node) {
function reportImpliedEvalCallExpression(node) {
const [firstArgument] = node.arguments;

// make sure our parent is marked
return node.parent === last(last(impliedEvalAncestorsStack)) &&
if (firstArgument) {

const staticValue = getStaticValue(firstArgument, context.getScope());
const isStaticString = staticValue && typeof staticValue.value === "string";
const isString = isStaticString || isEvaluatedString(firstArgument);

if (isString) {
context.report({
node,
messageId: "impliedEval"
});
}
}

// if our parent is a CallExpression, make sure we're the first argument
(node.parent.type !== "CallExpression" || node === node.parent.arguments[0]);
}

/**
* Checks if our parent is marked as part of an implied eval argument. If
* so, collapses the top of impliedEvalAncestorsStack and reports on the
* original CallExpression.
* @param {ASTNode} node The CallExpression to check.
* @returns {boolean} True if the node matches, false if not.
* @private
* Reports calls of `implied eval` via the global references.
* @param {Variable} globalVar A global variable to check.
* @returns {void}
*/
function checkString(node) {
if (hasImpliedEvalParent(node)) {
function reportImpliedEvalViaGlobal(globalVar) {
const { references, name } = globalVar;

// remove the entire substack, to avoid duplicate reports
const substack = impliedEvalAncestorsStack.pop();
references.forEach(ref => {
const identifier = ref.identifier;
let node = identifier.parent;

context.report({
node: substack[0],
messageId: "impliedEval"
});
}
while (isSpecifiedMember(node, [name])) {
node = node.parent;
}

if (isSpecifiedMember(node, EVAL_LIKE_FUNCS)) {
const parent = node.parent;

if (parent.type === "CallExpression" && parent.callee === node) {
reportImpliedEvalCallExpression(parent);
}
}
});
}

//--------------------------------------------------------------------------
Expand All @@ -124,45 +134,17 @@ module.exports = {

return {
CallExpression(node) {
if (isImpliedEvalCallExpression(node)) {

// call expressions create a new substack
impliedEvalAncestorsStack.push([node]);
}
},

"CallExpression:exit"(node) {
if (node === last(last(impliedEvalAncestorsStack))) {

/*
* Destroys the entire sub-stack, rather than just using
* last(impliedEvalAncestorsStack).pop(), as a CallExpression is
* always the bottom of a impliedEvalAncestorsStack substack.
*/
impliedEvalAncestorsStack.pop();
}
},

BinaryExpression(node) {
if (node.operator === "+" && hasImpliedEvalParent(node)) {
last(impliedEvalAncestorsStack).push(node);
}
},

"BinaryExpression:exit"(node) {
if (node === last(last(impliedEvalAncestorsStack))) {
last(impliedEvalAncestorsStack).pop();
}
},

Literal(node) {
if (typeof node.value === "string") {
checkString(node);
if (isSpecifiedIdentifier(node.callee, EVAL_LIKE_FUNCS)) {
reportImpliedEvalCallExpression(node);
}
},
"Program:exit"() {
const globalScope = context.getScope();

TemplateLiteral(node) {
checkString(node);
GLOBAL_CANDIDATES
.map(candidate => astUtils.getVariableByName(globalScope, candidate))
.filter(globalVar => !!globalVar && globalVar.defs.length === 0)
.forEach(reportImpliedEvalViaGlobal);
}
};

Expand Down

0 comments on commit 085979f

Please sign in to comment.