Merge pull request #13478 from aschackmull/java/varcapture

Java: Add proper support for variable capture flow.

java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll

@@ -101,6 +101,7 @@ abstract class SyntheticCallable extends string {
  * A module for importing frameworks that define synthetic callables.
  */
 private module SyntheticCallables {
+  private import semmle.code.java.dispatch.WrappedInvocation
   private import semmle.code.java.frameworks.android.Intent
   private import semmle.code.java.frameworks.Stream
 }
@@ -170,6 +171,8 @@ class SummarizedCallableBase extends TSummarizedCallableBase {
   }
 }
 
+class Provenance = Impl::Public::Provenance;
+
 class SummarizedCallable = Impl::Public::SummarizedCallable;
 
 class NeutralCallable = Impl::Public::NeutralCallable;

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

@@ -55,7 +55,8 @@ private module Cached {
       )
     } or
     TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
-    TFieldValueNode(Field f)
+    TFieldValueNode(Field f) or
+    TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn)
 
   cached
   newtype TContent =
@@ -64,6 +65,7 @@ private module Cached {
     TCollectionContent() or
     TMapKeyContent() or
     TMapValueContent() or
+    TCapturedVariableContent(CapturedVariable v) or
     TSyntheticFieldContent(SyntheticField s)
 
   cached
@@ -73,6 +75,7 @@ private module Cached {
     TCollectionContentApprox() or
     TMapKeyContentApprox() or
     TMapValueContentApprox() or
+    TCapturedVariableContentApprox(CapturedVariable v) or
     TSyntheticFieldApproxContent()
 }
 
@@ -127,6 +130,8 @@ module Public {
       or
       result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getType()
       or
+      result = this.(CaptureNode).getTypeImpl()
+      or
       result = this.(FieldValueNode).getField().getType()
     }
 
@@ -372,6 +377,7 @@ module Private {
     result.asCallable() = n.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
     result = nodeGetEnclosingCallable(n.(ImplicitPostUpdateNode).getPreUpdateNode()) or
     result.asSummarizedCallable() = n.(FlowSummaryNode).getSummarizedCallable() or
+    result.asCallable() = n.(CaptureNode).getSynthesizedCaptureNode().getEnclosingCallable() or
     result.asFieldScope() = n.(FieldValueNode).getField()
   }
 
@@ -491,6 +497,28 @@ module Private {
       c.asSummarizedCallable() = this.getSummarizedCallable() and pos = this.getPosition()
     }
   }
+
+  /**
+   * A synthesized data flow node representing a closure object that tracks
+   * captured variables.
+   */
+  class CaptureNode extends Node, TCaptureNode {
+    private CaptureFlow::SynthesizedCaptureNode cn;
+
+    CaptureNode() { this = TCaptureNode(cn) }
+
+    CaptureFlow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn }
+
+    override Location getLocation() { result = cn.getLocation() }
+
+    override string toString() { result = cn.toString() }
+
+    Type getTypeImpl() {
+      exists(Variable v | cn.isVariableAccess(v) and result = v.getType())
+      or
+      cn.isInstanceAccess() and result = cn.getEnclosingCallable().getDeclaringType()
+    }
+  }
 }
 
 private import Private
@@ -520,3 +548,14 @@ private class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
 
   override Node getPreUpdateNode() { result = pre }
 }
+
+private class CapturePostUpdateNode extends PostUpdateNode, CaptureNode {
+  private CaptureNode pre;
+
+  CapturePostUpdateNode() {
+    CaptureFlow::capturePostUpdateNode(this.getSynthesizedCaptureNode(),
+      pre.getSynthesizedCaptureNode())
+  }
+
+  override Node getPreUpdateNode() { result = pre }
+}

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -10,6 +10,7 @@ private import semmle.code.java.dataflow.FlowSummary
 private import FlowSummaryImpl as FlowSummaryImpl
 private import DataFlowImplConsistency
 private import DataFlowNodes
+private import codeql.dataflow.VariableCapture as VariableCapture
 import DataFlowNodes::Private
 
 private newtype TReturnKind = TNormalReturnKind()
@@ -51,26 +52,131 @@ private predicate fieldStep(Node node1, Node node2) {
   )
 }
 
-/**
- * Holds if data can flow from `node1` to `node2` through variable capture.
- */
-private predicate variableCaptureStep(Node node1, ExprNode node2) {
-  exists(SsaImplicitInit closure, SsaVariable captured |
-    closure.captures(captured) and
-    node2.getExpr() = closure.getAFirstUse()
-  |
-    node1.asExpr() = captured.getAUse()
-    or
-    not exists(captured.getAUse()) and
-    exists(SsaVariable capturedDef | capturedDef = captured.getAnUltimateDefinition() |
-      capturedDef.(SsaImplicitInit).isParameterDefinition(node1.asParameter()) or
-      capturedDef.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() =
-        node1.asExpr() or
-      capturedDef.(SsaExplicitUpdate).getDefiningExpr().(AssignOp) = node1.asExpr()
-    )
+private predicate closureFlowStep(Expr e1, Expr e2) {
+  simpleAstFlowStep(e1, e2)
+  or
+  exists(SsaVariable v |
+    v.getAUse() = e2 and
+    v.getAnUltimateDefinition().(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() =
+      e1
   )
 }
 
+private module CaptureInput implements VariableCapture::InputSig {
+  private import java as J
+
+  class Location = J::Location;
+
+  class BasicBlock instanceof J::BasicBlock {
+    string toString() { result = super.toString() }
+
+    Callable getEnclosingCallable() { result = super.getEnclosingCallable() }
+
+    Location getLocation() { result = super.getLocation() }
+  }
+
+  BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) }
+
+  BasicBlock getABasicBlockSuccessor(BasicBlock bb) {
+    result = bb.(J::BasicBlock).getABBSuccessor()
+  }
+
+  //TODO: support capture of `this` in lambdas
+  class CapturedVariable instanceof LocalScopeVariable {
+    CapturedVariable() {
+      2 <=
+        strictcount(J::Callable c |
+          c = this.getCallable() or c = this.getAnAccess().getEnclosingCallable()
+        )
+    }
+
+    string toString() { result = super.toString() }
+
+    Callable getCallable() { result = super.getCallable() }
+
+    Location getLocation() { result = super.getLocation() }
+  }
+
+  class CapturedParameter extends CapturedVariable instanceof Parameter { }
+
+  class Expr instanceof J::Expr {
+    string toString() { result = super.toString() }
+
+    Location getLocation() { result = super.getLocation() }
+
+    predicate hasCfgNode(BasicBlock bb, int i) { this = bb.(J::BasicBlock).getNode(i) }
+  }
+
+  class VariableWrite extends Expr instanceof VariableUpdate {
+    CapturedVariable v;
+
+    VariableWrite() { super.getDestVar() = v }
+
+    CapturedVariable getVariable() { result = v }
+
+    Expr getSource() {
+      result = this.(VariableAssign).getSource() or
+      result = this.(AssignOp)
+    }
+  }
+
+  class VariableRead extends Expr instanceof RValue {
+    CapturedVariable v;
+
+    VariableRead() { super.getVariable() = v }
+
+    CapturedVariable getVariable() { result = v }
+  }
+
+  class ClosureExpr extends Expr instanceof ClassInstanceExpr {
+    NestedClass nc;
+
+    ClosureExpr() {
+      nc.(AnonymousClass).getClassInstanceExpr() = this
+      or
+      nc instanceof LocalClass and
+      super.getConstructedType().getASourceSupertype*().getSourceDeclaration() = nc
+    }
+
+    predicate hasBody(Callable body) { nc.getACallable() = body }
+
+    predicate hasAliasedAccess(Expr f) { closureFlowStep+(this, f) and not closureFlowStep(f, _) }
+  }
+
+  class Callable extends J::Callable {
+    predicate isConstructor() { this instanceof Constructor }
+  }
+}
+
+class CapturedVariable = CaptureInput::CapturedVariable;
+
+class CapturedParameter = CaptureInput::CapturedParameter;
+
+module CaptureFlow = VariableCapture::Flow<CaptureInput>;
+
+private CaptureFlow::ClosureNode asClosureNode(Node n) {
+  result = n.(CaptureNode).getSynthesizedCaptureNode() or
+  result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
+  result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
+    n.(PostUpdateNode).getPreUpdateNode().asExpr() or
+  result.(CaptureFlow::ParameterNode).getParameter() = n.asParameter() or
+  result.(CaptureFlow::ThisParameterNode).getCallable() = n.(InstanceParameterNode).getCallable() or
+  exprNode(result.(CaptureFlow::MallocNode).getClosureExpr()).(PostUpdateNode).getPreUpdateNode() =
+    n
+}
+
+private predicate captureStoreStep(Node node1, CapturedVariableContent c, Node node2) {
+  CaptureFlow::storeStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+private predicate captureReadStep(Node node1, CapturedVariableContent c, Node node2) {
+  CaptureFlow::readStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+predicate captureValueStep(Node node1, Node node2) {
+  CaptureFlow::localFlowStep(asClosureNode(node1), asClosureNode(node2))
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` through a field or
  * variable capture.
@@ -78,10 +184,6 @@ private predicate variableCaptureStep(Node node1, ExprNode node2) {
 predicate jumpStep(Node node1, Node node2) {
   fieldStep(node1, node2)
   or
-  variableCaptureStep(node1, node2)
-  or
-  variableCaptureStep(node1.(PostUpdateNode).getPreUpdateNode(), node2)
-  or
   any(AdditionalValueStep a).step(node1, node2) and
   node1.getEnclosingCallable() != node2.getEnclosingCallable()
   or
@@ -117,6 +219,8 @@ predicate storeStep(Node node1, ContentSet f, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), f,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  captureStoreStep(node1, f, node2)
 }
 
 /**
@@ -149,6 +253,8 @@ predicate readStep(Node node1, ContentSet f, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), f,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  captureReadStep(node1, f, node2)
 }
 
 /**
@@ -231,19 +337,29 @@ private newtype TDataFlowCallable =
   TSummarizedCallable(SummarizedCallable c) or
   TFieldScope(Field f)
 
+/**
+ * A callable or scope enclosing some number of data flow nodes. This can either
+ * be a source callable, a synthesized callable for which we have a summary
+ * model, or a synthetic scope for a field value node.
+ */
 class DataFlowCallable extends TDataFlowCallable {
+  /** Gets the source callable corresponding to this callable, if any. */
   Callable asCallable() { this = TSrcCallable(result) }
 
+  /** Gets the summary model callable corresponding to this callable, if any. */
   SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) }
 
+  /** Gets the field corresponding to this callable, if it is a field value scope. */
   Field asFieldScope() { this = TFieldScope(result) }
 
+  /** Gets a textual representation of this callable. */
   string toString() {
     result = this.asCallable().toString() or
     result = "Synthetic: " + this.asSummarizedCallable().toString() or
     result = "Field scope: " + this.asFieldScope().toString()
   }
 
+  /** Gets the location of this callable. */
   Location getLocation() {
     result = this.asCallable().getLocation() or
     result = this.asSummarizedCallable().getLocation() or
@@ -406,6 +522,8 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  */
 predicate allowParameterReturnInSelf(ParameterNode p) {
   FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(p)
+  or
+  CaptureFlow::heuristicAllowInstanceParameterReturnInSelf(p.(InstanceParameterNode).getCallable())
 }
 
 /** An approximated `Content`. */
@@ -447,6 +565,10 @@ ContentApprox getContentApprox(Content c) {
   or
   c instanceof MapValueContent and result = TMapValueContentApprox()
   or
+  exists(CapturedVariable v |
+    c = TCapturedVariableContent(v) and result = TCapturedVariableContentApprox(v)
+  )
+  or
   c instanceof SyntheticFieldContent and result = TSyntheticFieldApproxContent()
 }
 

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll

@@ -135,6 +135,30 @@ private module Cached {
 
 import Cached
 
+private predicate capturedVariableRead(Node n) {
+  n.asExpr().(RValue).getVariable() instanceof CapturedVariable
+}
+
+/**
+ * Holds if there is a data flow step from `e1` to `e2` that only steps from
+ * child to parent in the AST.
+ */
+predicate simpleAstFlowStep(Expr e1, Expr e2) {
+  e2.(CastingExpr).getExpr() = e1
+  or
+  e2.(ChooseExpr).getAResultExpr() = e1
+  or
+  e2.(AssignExpr).getSource() = e1
+  or
+  e2.(ArrayCreationExpr).getInit() = e1
+  or
+  e2 = any(StmtExpr stmtExpr | e1 = stmtExpr.getResultExpr())
+  or
+  e2 = any(NotNullExpr nne | e1 = nne.getExpr())
+  or
+  e2.(WhenExpr).getBranch(_).getAResult() = e1
+}
+
 private predicate simpleLocalFlowStep0(Node node1, Node node2) {
   TaintTrackingUtil::forceCachingInSameStage() and
   // Variable flow steps through adjacent def-use and use-use pairs.
@@ -142,39 +166,31 @@ private predicate simpleLocalFlowStep0(Node node1, Node node2) {
     upd.getDefiningExpr().(VariableAssign).getSource() = node1.asExpr() or
     upd.getDefiningExpr().(AssignOp) = node1.asExpr()
   |
-    node2.asExpr() = upd.getAFirstUse()
+    node2.asExpr() = upd.getAFirstUse() and
+    not capturedVariableRead(node2)
   )
   or
   exists(SsaImplicitInit init |
     init.isParameterDefinition(node1.asParameter()) and
-    node2.asExpr() = init.getAFirstUse()
+    node2.asExpr() = init.getAFirstUse() and
+    not capturedVariableRead(node2)
   )
   or
   adjacentUseUse(node1.asExpr(), node2.asExpr()) and
   not exists(FieldRead fr |
     hasNonlocalValue(fr) and fr.getField().isStatic() and fr = node1.asExpr()
   ) and
-  not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(node1, _)
+  not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(node1, _) and
+  not capturedVariableRead(node2)
   or
   ThisFlow::adjacentThisRefs(node1, node2)
   or
-  adjacentUseUse(node1.(PostUpdateNode).getPreUpdateNode().asExpr(), node2.asExpr())
+  adjacentUseUse(node1.(PostUpdateNode).getPreUpdateNode().asExpr(), node2.asExpr()) and
+  not capturedVariableRead(node2)
   or
   ThisFlow::adjacentThisRefs(node1.(PostUpdateNode).getPreUpdateNode(), node2)
   or
-  node2.asExpr().(CastingExpr).getExpr() = node1.asExpr()
-  or
-  node2.asExpr().(ChooseExpr).getAResultExpr() = node1.asExpr()
-  or
-  node2.asExpr().(AssignExpr).getSource() = node1.asExpr()
-  or
-  node2.asExpr().(ArrayCreationExpr).getInit() = node1.asExpr()
-  or
-  node2.asExpr() = any(StmtExpr stmtExpr | node1.asExpr() = stmtExpr.getResultExpr())
-  or
-  node2.asExpr() = any(NotNullExpr nne | node1.asExpr() = nne.getExpr())
-  or
-  node2.asExpr().(WhenExpr).getBranch(_).getAResult() = node1.asExpr()
+  simpleAstFlowStep(node1.asExpr(), node2.asExpr())
   or
   exists(MethodAccess ma, ValuePreservingMethod m, int argNo |
     ma.getCallee().getSourceDeclaration() = m and m.returnsValue(argNo)
@@ -185,6 +201,8 @@ private predicate simpleLocalFlowStep0(Node node1, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryLocalStep(node1.(FlowSummaryNode).getSummaryNode(),
     node2.(FlowSummaryNode).getSummaryNode(), true)
+  or
+  captureValueStep(node1, node2)
 }
 
 /**
@@ -256,6 +274,19 @@ class MapValueContent extends Content, TMapValueContent {
   override string toString() { result = "<map.value>" }
 }
 
+/** A captured variable. */
+class CapturedVariableContent extends Content, TCapturedVariableContent {
+  CapturedVariable v;
+
+  CapturedVariableContent() { this = TCapturedVariableContent(v) }
+
+  CapturedVariable getVariable() { result = v }
+
+  override DataFlowType getType() { result = getErasedRepr(v.(Variable).getType()) }
+
+  override string toString() { result = v.toString() }
+}
+
 /** A reference through a synthetic instance field. */
 class SyntheticFieldContent extends Content, TSyntheticFieldContent {
   SyntheticField s;