JS: Handle multiple and/or operators in SanitizerFunction

Author: asger-semmle

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -846,6 +846,26 @@ module TaintTracking {
     override predicate appliesTo(Configuration cfg) { any() }
   }
 
+  /**
+   * Gets an operand of the given `&&` operator.
+   *
+   * We use this to construct the transitive closure over a relation
+   * that does not include all of `BinaryExpr.getAnOperand`.
+   */
+  private Expr getALogicalAndOperand(LogAndExpr e) {
+    result = e.getAnOperand()
+  }
+
+  /**
+   * Gets an operand of the given `||` operator.
+   *
+   * We use this to construct the transitive closure over a relation
+   * that does not include all of `BinaryExpr.getAnOperand`.
+   */
+  private Expr getALogicalOrOperand(LogOrExpr e) {
+    result = e.getAnOperand()
+  }
+
   /**
    * A function that returns the result of a sanitizer check.
    */
@@ -860,10 +880,10 @@ module TaintTracking {
           returnExpr = sanitizer.asExpr()
           or
           // ad hoc support for conjunctions:
-          returnExpr.(LogAndExpr).getAnOperand() = sanitizer.asExpr() and sanitizerOutcome = true
+          getALogicalAndOperand+(returnExpr) = sanitizer.asExpr() and sanitizerOutcome = true
           or
           // ad hoc support for disjunctions:
-          returnExpr.(LogOrExpr).getAnOperand() = sanitizer.asExpr() and sanitizerOutcome = false
+          getALogicalOrOperand+(returnExpr) = sanitizer.asExpr() and sanitizerOutcome = false
         |
           exists(SsaExplicitDefinition ssa |
             ssa.getDef().getSource() = returnExpr and

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -69,6 +69,8 @@ typeInferenceMismatch
 | promise.js:5:25:5:32 | source() | promise.js:5:8:5:33 | bluebir ... urce()) |
 | promise.js:10:24:10:31 | source() | promise.js:10:8:10:32 | Promise ... urce()) |
 | promise.js:12:20:12:27 | source() | promise.js:13:8:13:23 | resolver.promise |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:14:10:14:14 | taint |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:33:14:33:18 | taint |
 | sanitizer-guards.js:2:11:2:18 | source() | sanitizer-guards.js:4:8:4:8 | x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:15:10:15:15 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:21:14:21:19 | this.x |

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

@@ -41,6 +41,11 @@
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:41:10:41:18 | id(taint) |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:14:10:14:14 | taint |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:17:14:17:18 | taint |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:21:14:21:18 | taint |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:25:14:25:18 | taint |
+| sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:33:14:33:18 | taint |
 | sanitizer-guards.js:2:11:2:18 | source() | sanitizer-guards.js:4:8:4:8 | x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:15:10:15:15 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:21:14:21:19 | this.x |

javascript/ql/test/library-tests/TaintTracking/sanitizer-function.js

@@ -0,0 +1,35 @@
+function test() {
+    function myCheck1(x) {
+        return x === "a" && something() && somethingElse();
+    }
+    function myCheck2(x) {
+        return something() && x === "a" && somethingElse();
+    }
+    function myCheck3(x) {
+        return something() && somethingElse() && x === "a";
+    }
+
+    let taint = source();
+
+    sink(taint); // NOT OK
+
+    if (myCheck1(taint)) {
+        sink(taint); // OK
+    }
+
+    if (myCheck2(taint)) {
+        sink(taint); // OK
+    }
+
+    if (myCheck3(taint)) {
+        sink(taint); // OK
+    }
+
+    function badCheck(x) {
+        return something && x + isSafe(x) != null;
+    }
+
+    if (badCheck(taint)) {
+        sink(taint); // NOT OK
+    }
+}