Merge pull request #741 from asger-semmle/this-access-path

JS: support 'this' as the root of an access path

Author: Esben Sparre Andreasen

javascript/ql/src/semmle/javascript/dataflow/internal/AccessPaths.qll

@@ -44,7 +44,8 @@ private PropertyName getPropertyName(PropAccess pacc) {
  * where each property name is either constant or itself an SSA variable.
  */
 private newtype TAccessPath =
-  MkRoot(SsaVariable var) or
+  MkSsaRoot(SsaVariable var) or
+  MkThisRoot(Function function) { function.getThisBinder() = function } or
   MkAccessStep(AccessPath base, PropertyName name) {
     exists(PropAccess pacc |
       pacc.getBase() = base.getAnInstance() and
@@ -62,10 +63,16 @@ class AccessPath extends TAccessPath {
    */
   Expr getAnInstanceIn(BasicBlock bb) {
     exists(SsaVariable var |
-      this = MkRoot(var) and
+      this = MkSsaRoot(var) and
       result = var.getAUseIn(bb)
     )
     or
+    exists(ThisExpr this_ |
+      this = MkThisRoot(this_.getBinder()) and
+      result = this_ and
+      this_.getBasicBlock() = bb
+    )
+    or
     exists(PropertyName name |
       result = getABaseInstanceIn(bb, name) and
       getPropertyName(result) = name
@@ -96,7 +103,9 @@ class AccessPath extends TAccessPath {
    * Gets a textual representation of this access path.
    */
   string toString() {
-    exists(SsaVariable var | this = MkRoot(var) | result = var.getSourceVariable().getName())
+    exists(SsaVariable var | this = MkSsaRoot(var) | result = var.getSourceVariable().getName())
+    or
+    this = MkThisRoot(_) and result = "this"
     or
     exists(AccessPath base, PropertyName name, string rest |
       rest = "." + any(string s | name = StaticPropertyName(s))

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

@@ -12,6 +12,10 @@
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
 | promise.js:4:24:4:31 | source() | promise.js:4:8:4:32 | Promise ... urce()) |
 | promise.js:5:25:5:32 | source() | promise.js:5:8:5:33 | bluebir ... urce()) |
+| 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 |
+| sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:26:9:26:14 | this.x |
 | thisAssignments.js:4:17:4:24 | source() | thisAssignments.js:5:10:5:18 | obj.field |
 | thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
 | tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |

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

@@ -8,6 +8,18 @@ class BasicConfig extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node node) { node = getACall("source") }
 
   override predicate isSink(DataFlow::Node node) { node = getACall("sink").getAnArgument() }
+
+  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
+    node instanceof BasicSanitizerGuard
+  }
+}
+
+class BasicSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
+  BasicSanitizerGuard() { this = getACall("isSafe") }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    outcome = true and e = getArgument(0).asExpr()
+  }
 }
 
 from BasicConfig cfg, DataFlow::Node src, DataFlow::Node sink

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

@@ -0,0 +1,40 @@
+function test() {
+  let x = source();
+  
+  sink(x); // NOT OK
+  
+  if (isSafe(x)) {
+    sink(x); // OK
+  }
+}
+
+class C {
+  method() {
+    this.x = source();
+
+    sink(this.x); // NOT OK
+
+    if (isSafe(this.x)) {
+      sink(this.x); // OK
+
+      addEventListener('hey', () => {
+        sink(this.x); // OK - but still flagged
+      });
+    }
+
+    addEventListener('hey', () => {
+	  sink(this.x); // NOT OK
+	});
+
+	let self = this;
+	if (isSafe(self.x)) {
+	  sink(self.x); // OK
+	}
+
+	addEventListener('hey', function() {
+	  if (isSafe(self.x)) {
+	    sink(self.x); // OK
+	  }
+	});
+  }
+}