Merge pull request #4401 from asgerf/js/angular-prerequisites

Approved by erik-krogh

Author: codeql-ci

javascript/ql/src/semmle/javascript/DOM.qll

@@ -347,7 +347,11 @@ module DOM {
   }
 
   /** Gets a data flow node that may refer to a value from the DOM. */
-  DataFlow::SourceNode domValueRef() { result = domValueRef(DataFlow::TypeTracker::end()) }
+  DataFlow::SourceNode domValueRef() {
+    result = domValueRef(DataFlow::TypeTracker::end())
+    or
+    result.hasUnderlyingType("Element")
+  }
 
   module LocationSource {
     /**
@@ -419,5 +423,9 @@ module DOM {
   /**
    * Gets a reference to the 'document' object.
    */
-  DataFlow::SourceNode documentRef() { result = documentRef(DataFlow::TypeTracker::end()) }
+  DataFlow::SourceNode documentRef() {
+    result = documentRef(DataFlow::TypeTracker::end())
+    or
+    result.hasUnderlyingType("Document")
+  }
 }

javascript/ql/src/semmle/javascript/Extend.qll

@@ -34,6 +34,13 @@ abstract class ExtendCall extends DataFlow::CallNode {
   }
 }
 
+/** A version of `JQuery::dollarSource()` with fewer dependencies. */
+private DataFlow::SourceNode localDollar() {
+  result.accessesGlobal(["$", "jQuery"])
+  or
+  result = DataFlow::moduleImport("jquery")
+}
+
 /**
  * An extend call of form `extend(true/false, dst, src1, src2, ...)`, where the true/false
  * argument is possibly omitted.
@@ -47,9 +54,7 @@ private class ExtendCallWithFlag extends ExtendCall {
       name = "node.extend"
     )
     or
-    // Match $.extend using the source of `$` only, as ExtendCall should not
-    // depend on type tracking.
-    this = JQuery::dollarSource().getAMemberCall("extend")
+    this = localDollar().getAMemberCall("extend")
   }
 
   /**

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

@@ -420,6 +420,24 @@ private predicate barrierGuardBlocksSsaRefinement(
   )
 }
 
+/**
+ * Holds if the result of `guard` is used in the branching condition `cond`.
+ *
+ * `outcome` is bound to the outcome of `cond` for join-ordering purposes.
+ */
+pragma[noinline]
+private predicate barrierGuardUsedInCondition(
+  BarrierGuardNode guard, ConditionGuardNode cond, boolean outcome
+) {
+  barrierGuardIsRelevant(guard) and
+  outcome = cond.getOutcome() and
+  (
+    cond.getTest() = guard.getEnclosingExpr()
+    or
+    cond.getTest().flow().getImmediatePredecessor+() = guard
+  )
+}
+
 /**
  * Holds if data flow node `nd` acts as a barrier for data flow, possibly due to aliasing
  * through an access path.
@@ -435,14 +453,9 @@ private predicate barrierGuardBlocksNode(BarrierGuardNode guard, DataFlow::Node
   )
   or
   // 2) `nd` is an instance of an access path `p`, and dominated by a barrier for `p`
-  barrierGuardIsRelevant(guard) and
   exists(AccessPath p, BasicBlock bb, ConditionGuardNode cond, boolean outcome |
     nd = DataFlow::valueNode(p.getAnInstanceIn(bb)) and
-    (
-      guard.getEnclosingExpr() = cond.getTest() or
-      guard = cond.getTest().flow().getImmediatePredecessor+()
-    ) and
-    outcome = cond.getOutcome() and
+    barrierGuardUsedInCondition(guard, cond, outcome) and
     barrierGuardBlocksAccessPath(guard, outcome, p, label) and
     cond.dominates(bb)
   )

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

@@ -228,14 +228,19 @@ module DataFlow {
      *
      * Doesn't take field types and function return types into account.
      */
-    private JSDocTypeExpr getFallbackTypeAnnotation() {
+    private TypeAnnotation getFallbackTypeAnnotation() {
       exists(BindingPattern pattern |
         this = valueNode(pattern) and
         not ast_node_type(pattern, _) and
         result = pattern.getTypeAnnotation()
       )
       or
       result = getAPredecessor().getFallbackTypeAnnotation()
+      or
+      exists(DataFlow::ClassNode cls, string fieldName |
+        this = cls.getAReceiverNode().getAPropertyRead(fieldName) and
+        result = cls.getFieldTypeAnnotation(fieldName)
+      )
     }
 
     /**
@@ -704,7 +709,9 @@ module DataFlow {
       result = thisNode(prop.getDeclaringClass().getConstructor().getBody())
     }
 
-    override Expr getPropertyNameExpr() { result = prop.getNameExpr() }
+    override Expr getPropertyNameExpr() {
+      none() // The parameter value is not the name of the field
+    }
 
     override string getPropertyName() { result = prop.getName() }
 

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

@@ -995,6 +995,13 @@ class ClassNode extends DataFlow::SourceNode {
   predicate hasQualifiedName(string name) {
     getAClassReference().flowsTo(AccessPath::getAnAssignmentTo(name))
   }
+
+  /**
+   * Gets the type annotation for the field `fieldName`, if any.
+   */
+  TypeAnnotation getFieldTypeAnnotation(string fieldName) {
+    result = impl.getFieldTypeAnnotation(fieldName)
+  }
 }
 
 module ClassNode {
@@ -1047,6 +1054,11 @@ module ClassNode {
      * of this node.
      */
     abstract DataFlow::Node getASuperClassNode();
+
+    /**
+     * Gets the type annotation for the field `fieldName`, if any.
+     */
+    TypeAnnotation getFieldTypeAnnotation(string fieldName) { none() }
   }
 
   /**
@@ -1106,6 +1118,14 @@ module ClassNode {
     }
 
     override DataFlow::Node getASuperClassNode() { result = astNode.getSuperClass().flow() }
+
+    override TypeAnnotation getFieldTypeAnnotation(string fieldName) {
+      exists(FieldDeclaration field |
+        field.getDeclaringClass() = astNode and
+        fieldName = field.getName() and
+        result = field.getTypeAnnotation()
+      )
+    }
   }
 
   private DataFlow::PropRef getAPrototypeReferenceInFile(string name, File f) {

javascript/ql/test/library-tests/ClassNode/InstanceMember.expected

@@ -1,4 +1,4 @@
-| fields.ts:2:16:2:32 | (x: string) => {} | Foo.m | method |
+| fields.ts:12:16:12:32 | (x: string) => {} | Foo.m | method |
 | namespace.js:5:32:5:44 | function() {} | Baz.method | method |
 | tst2.js:6:9:9:3 | () {\\n   ... .x;\\n  } | C.method | method |
 | tst2.js:11:13:13:3 | () {\\n   ... .x;\\n  } | C.getter | getter |

javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected

@@ -1,4 +1,4 @@
-| fields.ts:2:16:2:32 | (x: string) => {} | Foo.m |
+| fields.ts:12:16:12:32 | (x: string) => {} | Foo.m |
 | namespace.js:5:32:5:44 | function() {} | Baz.method |
 | tst2.js:6:9:9:3 | () {\\n   ... .x;\\n  } | C.method |
 | tst2.js:18:14:18:22 | (x) => {} | D.f |

javascript/ql/test/library-tests/ClassNode/SuperClass.expected

@@ -1,3 +1,4 @@
+| fields.ts:5:1:13:1 | class F ... > {};\\n} | Foo | fields.ts:1:1:3:1 | class B ... mber;\\n} | Base |
 | tst.js:13:1:13:21 | class A ... ds A {} | A2 | tst.js:3:1:10:1 | class A ... () {}\\n} | A |
 | tst.js:15:1:15:15 | function B() {} | B | tst.js:3:1:10:1 | class A ... () {}\\n} | A |
 | tst.js:19:1:19:15 | function C() {} | C | tst.js:15:1:15:15 | function B() {} | B |

javascript/ql/test/library-tests/ClassNode/fields.ts

@@ -1,3 +1,13 @@
-class Foo {
+class Base {
+    baseField: number;
+}
+
+class Foo extends Base {
+    constructor(public x: number, private y: string) {
+        super();
+    }
+
+    z: string[];
+
     public m = (x: string) => {};
 }

javascript/ql/test/library-tests/ClassNode/getAReceiverNode.expected

@@ -1,4 +1,5 @@
-| fields.ts:1:1:3:1 | class F ... > {};\\n} | fields.ts:1:11:1:10 | this |
+| fields.ts:1:1:3:1 | class B ... mber;\\n} | fields.ts:1:12:1:11 | this |
+| fields.ts:5:1:13:1 | class F ... > {};\\n} | fields.ts:6:5:6:4 | this |
 | namespace.js:3:15:3:31 | function Baz() {} | namespace.js:3:15:3:14 | this |
 | namespace.js:3:15:3:31 | function Baz() {} | namespace.js:5:32:5:31 | this |
 | tst2.js:1:1:14:1 | class C ... ;\\n  }\\n} | tst2.js:2:14:2:13 | this |