diff --git a/lib/ruleHelper.js b/lib/ruleHelper.js index e711bc4..a859ce8 100644 --- a/lib/ruleHelper.js +++ b/lib/ruleHelper.js @@ -243,6 +243,19 @@ RuleHelper.prototype = { normalizeMethodCall(node) { let methodName; let objectName; + + // operators that allow us to only inspect the right side of the assignment: + const RIGHT_OPERATORS = ["="]; + + // operators where we can skip analysis: + // e.g., there's no point in doing a check if + // this ends up doing a mathematical operation. The result is not going to be a callable + const MATH_OPERATORS = ["+=", "-=", "*=", "/=", "%=", "**=", "<<=", ">>=", + ">>>=", "&=", "|=", "^="]; + + // operators where we don"t know the result of a logical expression, which may result in the code executing the left or the right part of the assignmentcan check the left bit: + const LOGICAL_OPERATOS = ["||=", "&&=", "??="]; + switch (node.type) { case "Identifier": methodName = node.name; @@ -256,6 +269,22 @@ RuleHelper.prototype = { case "ArrowFunctionExpression": methodName = ""; break; + case "AssignmentExpression": + if (RIGHT_OPERATORS.includes(node.operator)) { + methodName = this.normalizeMethodCall(node.right); + } else if (MATH_OPERATORS.includes(node.operator)) { + methodName = ""; + break; + } else if (LOGICAL_OPERATOS.includes(node.operator)) { + // Issue #210: oh, no! we have two methods names we need to check, but this function + // may only return one. it's not immediately clear how to reconcile that with existing callers of normalizeMethodCall. + methodName = ""; + } + else { + // this is the forcing function for us to complain about and implement support if new JS operators come up. + this.reportUnsupported(node, "Unexpected callable", `unexpected assignment with operator '${node.operator}' in normalizeMethodCall`); + } + break; case "Import": methodName = "import"; break; diff --git a/tests/rules/method.js b/tests/rules/method.js index ca038d8..2ff01a1 100644 --- a/tests/rules/method.js +++ b/tests/rules/method.js @@ -360,7 +360,20 @@ eslintTester.run("method", rule, { }, { code: "x.setHTML(evil, { sanitizer: new Sanitizer()})" + }, + { + code: "(info.current = type)(child_ctx)", + }, + { + code: "(info.current = n.insertAdjacentHTML)('beforebegin', 'innocent')", + }, + { + code: "(false ||= n.insertAdjacentHTML)('beforebegin', 'innocent')", + }, + { + code: "(n.insertAdjacentHTML &&= false)('beforebegin', 'innocent')", } + ], // Examples of code that should trigger the rule @@ -946,6 +959,58 @@ eslintTester.run("method", rule, { ], parserOptions: { ecmaVersion: 6 } }, + { + code: "(info.current = n.insertAdjacentHTML)('beforebegin', c)", + errors: [ + { + message: /Unsafe call to n.insertAdjacentHTML for argument 1/, + type: "CallExpression" + } + ], + }, + { + + // The issue with this testcase is, that it might not actually lead to a call to insertAdjacentHTML. + code: "(false ||= n.insertAdjacentHTML)('beforebegin', evil)", + errors: [ + { + message: /Unsafe call to n.insertAdjacentHTML for argument 1/, + type: "CallExpression" + } + ], + }, + { + + // The issue with this testcase is, that it might not actually lead to a call to insertAdjacentHTML. + code: "(n.insertAdjacentHTML ||= false)('beforebegin', evil)", + errors: [ + { + message: /Unsafe call to n.insertAdjacentHTML for argument 1/, + type: "CallExpression" + } + ], + }, + { + + // The issue with this testcase is, that it might not actually lead to a call to insertAdjacentHTML. + code: "(false &&= n.insertAdjacentHTML)('beforebegin', evil)", + errors: [ + { + message: /Unsafe call to n.insertAdjacentHTML for argument 1/, + type: "CallExpression" + } + ], + }, + { + // The issue with this testcase is, that it might not actually lead to a call to insertAdjacentHTML. + code: "(n.insertAdjacentHTML &&= false)('beforebegin', evil)", + errors: [ + { + message: /Unsafe call to n.insertAdjacentHTML for argument 1/, + type: "CallExpression" + } + ], + } ] });