JS: add StringOps::StartsWith and StringOps::Includes

javascript/ql/src/javascript.qll

@@ -41,6 +41,7 @@ import semmle.javascript.SSA
 import semmle.javascript.StandardLibrary
 import semmle.javascript.Stmt
 import semmle.javascript.StringConcatenation
+import semmle.javascript.StringOps
 import semmle.javascript.Templates
 import semmle.javascript.Tokens
 import semmle.javascript.TypeScript

javascript/ql/src/semmle/javascript/StringOps.qll

@@ -0,0 +1,302 @@
+/**
+ * Provides classes and predicates for reasoning about string-manipulating expressions.
+ */
+import javascript
+
+module StringOps {
+
+  /**
+   * A expression that is equivalent to `A.startsWith(B)` or `!A.startsWith(B)`.
+   */
+  abstract class StartsWith extends DataFlow::Node {
+    /**
+     * Gets the `A` in `A.startsWith(B)`.
+     */
+    abstract DataFlow::Node getBaseString();
+
+    /**
+     * Gets the `B` in `A.startsWith(B)`.
+     */
+    abstract DataFlow::Node getSubstring();
+
+    /**
+     * Gets the polarity if the check.
+     *
+     * If the polarity is `false` the check returns `true` if the string does not start
+     * with the given substring.
+     */
+    boolean getPolarity() { result = true }
+  }
+
+  /**
+   * An expression of form `A.startsWith(B)`.
+   */
+  private class StartsWith_Native extends StartsWith, DataFlow::MethodCallNode {
+    StartsWith_Native() {
+      getMethodName() = "startsWith" and
+      getNumArgument() = 1
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = getReceiver()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = getArgument(0)
+    }
+  }
+
+  /**
+   * An expression of form `A.indexOf(B) === 0`.
+   */
+  private class StartsWith_IndexOfEquals extends StartsWith, DataFlow::ValueNode {
+    override EqualityTest astNode;
+    DataFlow::MethodCallNode indexOf;
+
+    StartsWith_IndexOfEquals() {
+      indexOf.getMethodName() = "indexOf" and
+      indexOf.getNumArgument() = 1 and
+      indexOf.flowsToExpr(astNode.getAnOperand()) and
+      astNode.getAnOperand().getIntValue() = 0
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = indexOf.getReceiver()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = indexOf.getArgument(0)
+    }
+
+    override boolean getPolarity() {
+      result = astNode.getPolarity()
+    }
+  }
+
+  /**
+   * An expression of form `A.indexOf(B)` coerced to a boolean.
+   *
+   * This is equivalent to `!A.startsWith(B)` since all return values other than zero map to `true`.
+   */
+  private class StartsWith_IndexOfCoercion extends StartsWith, DataFlow::MethodCallNode {
+    StartsWith_IndexOfCoercion() {
+      getMethodName() = "indexOf" and
+      getNumArgument() = 1 and
+      this.flowsToExpr(any(ConditionGuardNode guard).getTest()) // check for boolean coercion
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = getReceiver()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = getArgument(0)
+    }
+
+    override boolean getPolarity() {
+      result = false
+    }
+  }
+
+  /**
+   * A call of form `_.startsWith(A, B)` or `ramda.startsWith(A, B)`.
+   */
+  private class StartsWith_Library extends StartsWith, DataFlow::CallNode {
+    StartsWith_Library() {
+      getNumArgument() = 2 and
+      exists (DataFlow::SourceNode callee | this = callee.getACall() |
+        callee = LodashUnderscore::member("startsWith") or
+        callee = DataFlow::moduleMember("ramda", "startsWith")
+      )
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = getArgument(0)
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = getArgument(1)
+    }
+  }
+
+  /**
+   * A comparison of form `x[0] === "k"` for some single-character constant `k`.
+   */
+  private class StartsWith_FirstCharacter extends StartsWith, DataFlow::ValueNode {
+    override EqualityTest astNode;
+    DataFlow::PropRead read;
+    Expr constant;
+
+    StartsWith_FirstCharacter() {
+      read.flowsTo(astNode.getAnOperand().flow()) and
+      read.getPropertyNameExpr().getIntValue() = 0 and
+      constant.getStringValue().length() = 1 and
+      astNode.getAnOperand() = constant
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = read.getBase()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = constant.flow()
+    }
+
+    override boolean getPolarity() {
+      result = astNode.getPolarity()
+    }
+  }
+
+
+  /**
+   * A expression that is equivalent to `A.includes(B)` or `!A.includes(B)`.
+   *
+   * Note that this also includes calls to the array method named `includes`.
+   */
+  abstract class Includes extends DataFlow::Node {
+    /** Gets the `A` in `A.includes(B)`. */
+    abstract DataFlow::Node getBaseString();
+
+    /** Gets the `B` in `A.includes(B)`. */
+    abstract DataFlow::Node getSubstring();
+
+    /**
+     * Gets the polarity if the check.
+     *
+     * If the polarity is `false` the check returns `true` if the string does not start
+     * with the given substring.
+     */
+    boolean getPolarity() { result = true }
+  }
+
+  /**
+   * A call to a method named `includes`, assumed to refer to `String.prototype.includes`.
+   */
+  private class Includes_Native extends Includes, DataFlow::MethodCallNode {
+    Includes_Native() {
+      getMethodName() = "includes" and
+      getNumArgument() = 1
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = getReceiver()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = getArgument(0)
+    }
+  }
+
+  /**
+   * A call to `_.includes`, assumed to operate on strings.
+   */
+  private class Includes_Library extends Includes, DataFlow::CallNode {
+    Includes_Library() {
+      this = LodashUnderscore::member("includes").getACall()
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = getArgument(0)
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = getArgument(1)
+    }
+  }
+
+  /**
+   * A check of form `A.indexOf(B) !== -1` or similar.
+   */
+  private class Includes_IndexOfEquals extends Includes, DataFlow::ValueNode {
+    MethodCallExpr indexOf;
+    override EqualityTest astNode;
+
+    Includes_IndexOfEquals() {
+      exists (Expr index | astNode.hasOperands(indexOf, index) |
+        // one operand is of the form `whitelist.indexOf(x)`
+        indexOf.getMethodName() = "indexOf" and
+        // and the other one is -1
+        index.getIntValue() = -1
+      )
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = indexOf.getReceiver().flow()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = indexOf.getArgument(0).flow()
+    }
+
+    override boolean getPolarity() {
+      result = astNode.getPolarity().booleanNot()
+    }
+  }
+
+  /**
+   * A check of form `A.indexOf(B) >= 0` or similar.
+   */
+  private class Includes_IndexOfRelational extends Includes, DataFlow::ValueNode {
+    MethodCallExpr indexOf;
+    override RelationalComparison astNode;
+    boolean polarity;
+
+    Includes_IndexOfRelational() {
+      exists (Expr lesser, Expr greater |
+        astNode.getLesserOperand() = lesser and
+        astNode.getGreaterOperand() = greater and
+        indexOf.getMethodName() = "indexOf" and
+        indexOf.getNumArgument() = 1 |
+        polarity = true and
+        greater = indexOf and
+        (
+          lesser.getIntValue() >= 0
+          or
+          lesser.getIntValue() = -1 and not astNode.isInclusive()
+        )
+        or
+        polarity = false and
+        lesser = indexOf and
+        (
+          greater.getIntValue() = -1
+          or
+          greater.getIntValue() = 0 and not astNode.isInclusive()
+        )
+      )
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = indexOf.getReceiver().flow()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = indexOf.getArgument(0).flow()
+    }
+
+    override boolean getPolarity() {
+      result = polarity
+    }
+  }
+
+  /**
+   * An expression of form `~A.indexOf(B)` which, when coerced to a boolean, is equivalent to `A.includes(B)`.
+   */
+  private class Includes_IndexOfBitwise extends Includes, DataFlow::ValueNode {
+    MethodCallExpr indexOf;
+    override BitNotExpr astNode;
+
+    Includes_IndexOfBitwise() {
+      astNode.getOperand() = indexOf and
+      indexOf.getMethodName() = "indexOf"
+    }
+
+    override DataFlow::Node getBaseString() {
+      result = indexOf.getReceiver().flow()
+    }
+
+    override DataFlow::Node getSubstring() {
+      result = indexOf.getArgument(0).flow()
+    }
+  }
+
+}

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

@@ -593,7 +593,9 @@ module TaintTracking {
   /**
    * A check of the form `if(o.<contains>(x))`, which sanitizes `x` in its "then" branch.
    *
-   * `<contains>` is one of: `contains`, `has`, `hasOwnProperty`, `includes`
+   * `<contains>` is one of: `contains`, `has`, `hasOwnProperty`
+   *
+   * Note that the `includes` method is covered by `StringInclusionSanitizer`.
    */
   class WhitelistContainmentCallSanitizer extends AdditionalSanitizerGuardNode,
     DataFlow::MethodCallNode {
@@ -601,8 +603,7 @@ module TaintTracking {
       exists(string name |
         name = "contains" or
         name = "has" or
-        name = "hasOwnProperty" or
-        name = "includes"
+        name = "hasOwnProperty"
       |
         getMethodName() = name
       )
@@ -673,91 +674,15 @@ module TaintTracking {
     override predicate appliesTo(Configuration cfg) { any() }
   }
 
-  /** A check of the form `if(whitelist.indexOf(x) != -1)`, which sanitizes `x` in its "then" branch. */
-  class IndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
-    MethodCallExpr indexOf;
+  /** A check of the form `whitelist.includes(x)` or equivalent, which sanitizes `x` in its "then" branch. */
+  class StringInclusionSanitizer extends AdditionalSanitizerGuardNode {
+    StringOps::Includes includes;
 
-    override EqualityTest astNode;
-
-    IndexOfSanitizer() {
-      exists(Expr index | astNode.hasOperands(indexOf, index) |
-        // one operand is of the form `whitelist.indexOf(x)`
-        indexOf.getMethodName() = "indexOf" and
-        // and the other one is -1
-        index.getIntValue() = -1
-      )
-    }
+    StringInclusionSanitizer() { this = includes }
 
     override predicate sanitizes(boolean outcome, Expr e) {
-      outcome = astNode.getPolarity().booleanNot() and
-      e = indexOf.getArgument(0)
-    }
-
-    override predicate appliesTo(Configuration cfg) { any() }
-  }
-
-  /**
-   * A check of the form `if(whitelist.indexOf(x) >= 0)`, which sanitizes `x` in its "then" branch.
-   *
-   * Similar relational checks are also supported.
-   */
-  private class RelationalIndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
-    MethodCallExpr indexOf;
-
-    override RelationalComparison astNode;
-
-    boolean polarity;
-
-    RelationalIndexOfSanitizer() {
-      exists(Expr lesser, Expr greater |
-        astNode.getLesserOperand() = lesser and
-        astNode.getGreaterOperand() = greater and
-        indexOf.getMethodName() = "indexOf"
-      |
-        polarity = true and
-        greater = indexOf and
-        (
-          lesser.getIntValue() >= 0
-          or
-          lesser.getIntValue() = -1 and not astNode.isInclusive()
-        )
-        or
-        polarity = false and
-        lesser = indexOf and
-        (
-          greater.getIntValue() = -1
-          or
-          greater.getIntValue() = 0 and not astNode.isInclusive()
-        )
-      )
-    }
-
-    override predicate sanitizes(boolean outcome, Expr e) {
-      outcome = polarity and
-      e = indexOf.getArgument(0)
-    }
-
-    override predicate appliesTo(Configuration cfg) { any() }
-  }
-
-  /**
-   * A check of the form `if(~whitelist.indexOf(x))`, which sanitizes `x` in its "then" branch.
-   *
-   * This sanitizer is equivalent to `if(whitelist.indexOf(x) != -1)`, since `~n = 0` iff `n = -1`.
-   */
-  private class BitwiseIndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
-    MethodCallExpr indexOf;
-
-    override BitNotExpr astNode;
-
-    BitwiseIndexOfSanitizer() {
-      astNode.getOperand() = indexOf and
-      indexOf.getMethodName() = "indexOf"
-    }
-
-    override predicate sanitizes(boolean outcome, Expr e) {
-      outcome = true and
-      e = indexOf.getArgument(0)
+      outcome = includes.getPolarity() and
+      e = includes.getSubstring().asExpr()
     }
 
     override predicate appliesTo(Configuration cfg) { any() }

javascript/ql/test/library-tests/StringOps/StartsWith/StartsWith.expected

@@ -0,0 +1,10 @@
+| tst.js:5:9:5:23 | A.startsWith(B) | tst.js:5:9:5:9 | A | tst.js:5:22:5:22 | B | true |
+| tst.js:6:9:6:26 | _.startsWith(A, B) | tst.js:6:22:6:22 | A | tst.js:6:25:6:25 | B | true |
+| tst.js:7:9:7:26 | R.startsWith(A, B) | tst.js:7:22:7:22 | A | tst.js:7:25:7:25 | B | true |
+| tst.js:8:9:8:26 | A.indexOf(B) === 0 | tst.js:8:9:8:9 | A | tst.js:8:19:8:19 | B | true |
+| tst.js:9:9:9:26 | A.indexOf(B) !== 0 | tst.js:9:9:9:9 | A | tst.js:9:19:9:19 | B | false |
+| tst.js:10:9:10:26 | 0 !== A.indexOf(B) | tst.js:10:15:10:15 | A | tst.js:10:25:10:25 | B | false |
+| tst.js:11:9:11:25 | 0 != A.indexOf(B) | tst.js:11:14:11:14 | A | tst.js:11:24:11:24 | B | false |
+| tst.js:12:9:12:20 | A.indexOf(B) | tst.js:12:9:12:9 | A | tst.js:12:19:12:19 | B | false |
+| tst.js:13:10:13:21 | A.indexOf(B) | tst.js:13:10:13:10 | A | tst.js:13:20:13:20 | B | false |
+| tst.js:14:11:14:22 | A.indexOf(B) | tst.js:14:11:14:11 | A | tst.js:14:21:14:21 | B | false |

javascript/ql/test/library-tests/StringOps/StartsWith/StartsWith.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from StringOps::StartsWith check
+select check, check.getBaseString(), check.getSubstring(), check.getPolarity()

javascript/ql/test/library-tests/StringOps/StartsWith/tst.js

@@ -0,0 +1,25 @@
+import * as _ from 'lodash';
+import * as R from 'ramda';
+
+function f(A, B) {
+    if (A.startsWith(B)) {}
+    if (_.startsWith(A, B)) {}
+    if (R.startsWith(A, B)) {}
+    if (A.indexOf(B) === 0) {}
+    if (A.indexOf(B) !== 0) {}
+    if (0 !== A.indexOf(B)) {}
+    if (0 != A.indexOf(B)) {}
+    if (A.indexOf(B)) {}  // !startsWith
+    if (!A.indexOf(B)) {} // startsWith
+    if (!!A.indexOf(B)) {} // !startsWith
+
+    // non-examples
+    if (_.startsWith(A, B, 2)) {} 
+    if (A.indexOf(B) >= 0) {}
+    if (A.indexOf(B) === 1) {}
+    if (A.indexOf(B) === A.indexOf(B)) {}
+    if (A.indexOf(B) !== -1) {}
+    if (A.indexOf(B, 2) === 0) {}
+    if (A.indexOf(B, 2)) {}
+    if (~A.indexOf(B)) {} // checks for existence, not startsWith
+}