Merge pull request #15 from xiemaisi/js/call-graph-data-flow

JavaScript: Lift call graph library to data flow graph.

Author: Esben Sparre Andreasen

change-notes/1.18/analysis-javascript.md

@@ -50,3 +50,4 @@
 
 * HTTP header names are now always normalized to lower case to reflect the fact that they are case insensitive. In particular, the result of `HeaderDefinition.getAHeaderName`, and the first parameter of `HeaderDefinition.defines`, `ExplicitHeaderDefinition.definesExplicitly` and `RouteHandler.getAResponseHeader` is now always a lower-case string.
 * The class `JsonParseCall` has been deprecated. Use `JsonParserCall` instead.
+* The handling of spread arguments in the data flow library has been changed: `DataFlow::InvokeNode.getArgument(i)` is now only defined when there is no spread argument at or before argument position `i`, and similarly `InvokeNode.getNumArgument` is only defined for invocations without spread arguments.

javascript/ql/src/Expressions/ExprHasNoEffect.ql

@@ -121,8 +121,7 @@ predicate noSideEffects(Expr e) {
   e.isPure()
   or
   // `new Error(...)`, `new SyntaxError(...)`, etc.
-  e instanceof NewExpr and
-  forex (Function f | f = e.(CallSite).getACallee() |
+  forex (Function f | f = e.flow().(DataFlow::NewNode).getACallee() |
     f.(ExternalType).getASupertype*().getName() = "Error"
   )
 }

javascript/ql/src/LanguageFeatures/IllegalInvocation.ql

@@ -16,13 +16,13 @@ import javascript
 /**
  * Holds if call site `cs` may invoke function `callee` as specified by `how`.
  */
-predicate calls(CallSite cs, Function callee, string how) {
+predicate calls(DataFlow::InvokeNode cs, Function callee, string how) {
   callee = cs.getACallee() and
   (
-    cs instanceof CallExpr and not cs instanceof SuperCall and
+    cs instanceof DataFlow::CallNode and not cs.asExpr() instanceof SuperCall and
     how = "as a function"
     or
-    cs instanceof NewExpr and
+    cs instanceof DataFlow::NewNode and
     how = "using 'new'"
   )
 }
@@ -31,7 +31,7 @@ predicate calls(CallSite cs, Function callee, string how) {
  * Holds if call site `cs` may illegally invoke function `callee` as specified by `how`;
  * `calleeDesc` describes what kind of function `callee` is.
  */
-predicate illegalInvocation(CallSite cs, Function callee, string calleeDesc, string how) {
+predicate illegalInvocation(DataFlow::InvokeNode cs, Function callee, string calleeDesc, string how) {
   calls(cs, callee, how) and
   (
     how = "as a function" and
@@ -44,15 +44,16 @@ predicate illegalInvocation(CallSite cs, Function callee, string calleeDesc, str
 }
 
 /**
- * Holds if `ce` has at least one call target that isn't a constructor.
+ * Holds if `ce` is a call with at least one call target that isn't a constructor.
  */
-predicate isCallToFunction(CallExpr ce) {
-  exists (Function f | f = ce.(CallSite).getACallee() |
+predicate isCallToFunction(DataFlow::InvokeNode ce) {
+  ce instanceof DataFlow::CallNode and
+  exists (Function f | f = ce.getACallee() |
     not f instanceof Constructor
   )
 }
 
-from CallSite cs, Function callee, string calleeDesc, string how
+from DataFlow::InvokeNode cs, Function callee, string calleeDesc, string how
 where illegalInvocation(cs, callee, calleeDesc, how) and
       // filter out some easy cases
       not isCallToFunction(cs) and

javascript/ql/src/LanguageFeatures/InconsistentNew.ql

@@ -22,8 +22,8 @@ import semmle.javascript.RestrictedLocations
  * have to call itself using `new`, so that is what we look for.
  */
 predicate guardsAgainstMissingNew(Function f) {
-  exists (CallSite new |
-    new.(NewExpr).getEnclosingFunction() = f and
+  exists (DataFlow::NewNode new |
+    new.asExpr().getEnclosingFunction() = f and
     f = new.getACallee()
   )
 }
@@ -34,13 +34,13 @@ predicate guardsAgainstMissingNew(Function f) {
  * is only suggested as a potential callee due to imprecise analysis of global
  * variables and is not, in fact, a viable callee at all.
  */
-predicate calls(CallSite cs, Function callee, int imprecision) {
+predicate calls(DataFlow::InvokeNode cs, Function callee, int imprecision) {
   callee = cs.getACallee() and
   (
     // if global flow was used to derive the callee, we may be imprecise
     if cs.isIndefinite("global") then
       // callees within the same file are probably genuine
-      callee.getFile() = cs.(Locatable).getFile() and imprecision = 0
+      callee.getFile() = cs.getFile() and imprecision = 0
       or
       // calls to global functions declared in an externs file are fairly
       // safe as well
@@ -58,11 +58,11 @@ predicate calls(CallSite cs, Function callee, int imprecision) {
  * Gets a function that may be invoked at `cs`, preferring callees that
  * are less likely to be derived due to analysis imprecision.
  */
-Function getALikelyCallee(CallSite cs) {
+Function getALikelyCallee(DataFlow::InvokeNode cs) {
   calls(cs, result, min(int p | calls(cs, _, p)))
 }
 
-from Function f, NewExpr new, CallExpr call
+from Function f, DataFlow::NewNode new, DataFlow::CallNode call
 where // externs are special, so don't flag them
       not f.inExternsFile() and
       // illegal constructor calls are flagged by query 'Illegal invocation',
@@ -71,11 +71,11 @@ where // externs are special, so don't flag them
       f = getALikelyCallee(new) and
       f = getALikelyCallee(call) and
       not guardsAgainstMissingNew(f) and
-      not new.(CallSite).isUncertain() and
-      not call.(CallSite).isUncertain() and
+      not new.isUncertain() and
+      not call.isUncertain() and
       // super constructor calls behave more like `new`, so don't flag them
-      not call instanceof SuperCall and
-      // reflective calls provide an explicit receiver object, so don't flag them either
-      not call instanceof ReflectiveCallSite
+      not call.asExpr() instanceof SuperCall and
+      // don't flag if there is a receiver object
+      not exists(call.getReceiver())
 select (FirstLineOf)f, capitalize(f.describe()) + " is invoked as a constructor here $@, " +
       "and as a normal function here $@.", new, new.toString(), call, call.toString()

javascript/ql/src/LanguageFeatures/SpuriousArguments.ql

@@ -29,23 +29,11 @@ predicate isFixedArity(Function fn) {
  *
  * This is only defined if all potential callees have a fixed arity.
  */
-int maxArity(CallSite invk) {
+int maxArity(DataFlow::InvokeNode invk) {
   forall (Function callee | callee = invk.getACallee() | isFixedArity(callee)) and
   result = max(invk.getACallee().getNumParameter())
 }
 
-/**
- * Holds if call site `invk` has more arguments than the maximum arity
- * of any function that it may invoke, and the first of those
- * arguments is `arg`.
- *
- * This predicate is only defined for call sites where callee information is complete.
- */
-predicate firstSpuriousArgument(CallSite invk, Expr arg) {
-  arg = invk.getArgumentNode(maxArity(invk)).asExpr() and
-  not invk.isIncomplete()
-}
-
 /**
  * A list of spurious arguments passed at a call site.
  *
@@ -54,15 +42,18 @@ predicate firstSpuriousArgument(CallSite invk, Expr arg) {
  * defined to cover all subsequent arguments as well.
  */
 class SpuriousArguments extends Expr {
+  DataFlow::InvokeNode invk;
+
   SpuriousArguments() {
-    firstSpuriousArgument(_, this)
+    this = invk.getArgument(maxArity(invk)).asExpr() and
+    not invk.isIncomplete()
   }
 
   /**
    * Gets the call site at which the spurious arguments are passed.
    */
-  CallSite getCall() {
-    firstSpuriousArgument(result, this)
+  DataFlow::InvokeNode getCall() {
+    result = invk
   }
 
   /**
@@ -71,7 +62,7 @@ class SpuriousArguments extends Expr {
    * expected by any potential callee.
    */
   int getCount() {
-    result = getCall().(InvokeExpr).getNumArgument() - maxArity(getCall())
+    result = count(int i | exists(invk.getArgument(i)) and i >= maxArity(getCall()))
   }
 
   /**
@@ -82,14 +73,15 @@ class SpuriousArguments extends Expr {
    * [LGTM locations](https://lgtm.com/help/ql/locations).
    */
   predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _) and
-    exists (Expr lastArg | lastArg = getCall().(InvokeExpr).getLastArgument() |
-      lastArg.getLocation().hasLocationInfo(_, _, _, endline, endcolumn)
+    getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _) and
+    exists (DataFlow::Node lastArg |
+      lastArg = max(DataFlow::Node arg, int i | arg = invk.getArgument(i) | arg order by i) |
+      lastArg.hasLocationInfo(_, _, _, endline, endcolumn)
     )
   }
 }
 
 from SpuriousArguments args, Function f, string arguments
 where f = args.getCall().getACallee() and
       if args.getCount() = 1 then arguments = "argument" else arguments = "arguments"
-select args, "Superfluous " + arguments + " passed to $@.", f, f.describe()
+select args, "Superfluous " + arguments + " passed to $@.", f, f.describe()

javascript/ql/src/React/UnsupportedStateUpdateInLifecycleMethod.ql

@@ -14,25 +14,25 @@ import javascript
 /**
  * A call that invokes a method on its own receiver.
  */
-class CallOnSelf extends CallExpr {
+class CallOnSelf extends DataFlow::CallNode {
 
   CallOnSelf() {
     exists (Function binder |
       binder = getEnclosingFunction().getThisBinder() |
       exists (DataFlow::ThisNode thiz |
-        this = thiz.getAMethodCall(_).asExpr() and
+        this = thiz.getAMethodCall(_) and
         thiz.getBinder().getAstNode() = binder
       )
       or
-      this.(CallSite).getACallee().(ArrowFunctionExpr).getThisBinder() = binder
+      this.getACallee().(ArrowFunctionExpr).getThisBinder() = binder
     )
   }
 
   /**
    * Gets a `CallOnSelf` in the callee of this call.
    */
   CallOnSelf getACalleCallOnSelf() {
-    result.getEnclosingFunction() = this.(CallSite).getACallee()
+    result.getEnclosingFunction() = this.getACallee()
   }
 
 }
@@ -55,15 +55,15 @@ class UnconditionalCallOnSelf extends CallOnSelf {
 /**
  * Holds if `call` is guaranteed to occur in its enclosing function, unless an exception occurs.
  */
-predicate isUnconditionalCall(CallExpr call) {
+predicate isUnconditionalCall(DataFlow::CallNode call) {
   exists (ReachableBasicBlock callBlock, ReachableBasicBlock entryBlock |
     callBlock.postDominates(entryBlock) and
-    callBlock.getANode() = call and
+    callBlock = call.getBasicBlock() and
     entryBlock = call.getEnclosingFunction().getEntryBB()
   )
 }
 
-predicate isStateUpdateMethodCall(MethodCallExpr mce) {
+predicate isStateUpdateMethodCall(DataFlow::MethodCallNode mce) {
   exists (string updateMethodName |
     updateMethodName = "setState" or
     updateMethodName = "replaceState" or
@@ -111,7 +111,8 @@ class StateUpdateVolatileMethod extends Function {
 
 }
 
-from StateUpdateVolatileMethod root, CallOnSelf initCall, MethodCallExpr stateUpdate, string callDescription
+from StateUpdateVolatileMethod root, CallOnSelf initCall, DataFlow::MethodCallNode stateUpdate,
+     string callDescription
 where initCall.getEnclosingFunction() = root and
       stateUpdate = initCall.getACalleCallOnSelf*() and
       isStateUpdateMethodCall(stateUpdate) and

javascript/ql/src/Statements/ImplicitReturn.ql

@@ -55,9 +55,9 @@ int numRet(Function f) {
  */
 predicate isDualUseConstructor(Function f) {
   numRet(f) = 1 and
-  exists (ReturnStmt ret, NewExpr new | ret.getContainer() = f |
-    new = ret.getExpr().stripParens() and
-    new.(CallSite).getACallee() = f
+  exists (ReturnStmt ret, DataFlow::NewNode new | ret.getContainer() = f |
+    new.asExpr() = ret.getExpr().stripParens() and
+    new.getACallee() = f
   )
 }
 

javascript/ql/src/semmle/javascript/StandardLibrary.qll

@@ -9,7 +9,7 @@ class CallToObjectDefineProperty extends DataFlow::MethodCallNode {
   CallToObjectDefineProperty() {
     exists (GlobalVariable obj |
       obj.getName() = "Object" and
-      astNode.calls(obj.getAnAccess(), "defineProperty")
+      calls(DataFlow::valueNode(obj.getAnAccess()), "defineProperty")
     )
   }
 

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

@@ -6,11 +6,13 @@ import javascript
 private import InferredTypes
 
 /**
+ * DEPRECATED: Use `DataFlow::InvokeNode` instead.
+ *
  * A function call or `new` expression, with information about its potential callees.
  *
  * Both direct calls and reflective calls using `call` or `apply` are modelled.
  */
-class CallSite extends @invokeexpr {
+deprecated class CallSite extends @invokeexpr {
   InvokeExpr invk;
 
   CallSite() { invk = this }
@@ -120,7 +122,7 @@ class CallSite extends @invokeexpr {
 /**
  * A reflective function call using `call` or `apply`.
  */
-class ReflectiveCallSite extends CallSite {
+deprecated class ReflectiveCallSite extends CallSite {
   DataFlow::AnalyzedNode callee;
   string callMode;
 

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

@@ -407,7 +407,7 @@ private predicate callInputStep(Function f, DataFlow::Node invk,
    or
    exists (SsaDefinition prevDef, SsaDefinition def |
      pred = DataFlow::ssaDefinitionNode(prevDef) and
-     calls(invk.asExpr(), f) and captures(f, prevDef, def) and
+     calls(invk, f) and captures(f, prevDef, def) and
      succ = DataFlow::ssaDefinitionNode(def)
    )
   ) and