wip: issue #210 and normalizeMethodcall

mozilla · Nov 1, 2022 · e176433 · e176433
1 parent 5f0b4a4
commit e176433
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 1 deletion.
diff --git 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;
@@ -257,7 +270,20 @@ RuleHelper.prototype = {
             methodName = "";
             break;
         case "AssignmentExpression":
-            methodName = this.normalizeMethodCall(node.right);
+            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";

diff --git a/tests/rules/method.js b/tests/rules/method.js
@@ -366,7 +366,14 @@ eslintTester.run("method", rule, {
         },
         {
             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
@@ -961,5 +968,49 @@ eslintTester.run("method", rule, {
                 }
             ],
         },
+        {
+
+            // 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"
+                }
+            ],
+        }
     ]
 });