C++: Data flow through reference parameters

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll

@@ -51,7 +51,9 @@ class ArgumentNode extends Node {
   DataFlowCall getCall() { this.argumentOf(result, _) }
 }
 
-private newtype TReturnKind = TNormalReturnKind()
+private newtype TReturnKind =
+  TNormalReturnKind() or
+  TRefReturnKind(int i) { exists(Parameter parameter | i = parameter.getIndex()) }
 
 /**
  * A return kind. A return kind describes how a value can be returned
@@ -59,23 +61,54 @@ private newtype TReturnKind = TNormalReturnKind()
  */
 class ReturnKind extends TReturnKind {
   /** Gets a textual representation of this return kind. */
-  string toString() { result = "return" }
+  string toString() {
+    this instanceof TNormalReturnKind and
+    result = "return"
+    or
+    this instanceof TRefReturnKind and
+    result = "ref"
+  }
 }
 
-/** A data flow node that occurs as the result of a `ReturnStmt`. */
-class ReturnNode extends ExprNode {
-  ReturnNode() { exists(ReturnStmt ret | this.getExpr() = ret.getExpr()) }
+/** A data flow node that represents a returned value. */
+abstract class ReturnNode extends Node {
+  /** Gets the kind of this returned value. */
+  abstract ReturnKind getKind();
+}
+
+/** A `ReturnNode` that occurs as the result of a `ReturnStmt`. */
+private class NormalReturnNode extends ReturnNode, ExprNode {
+  NormalReturnNode() { exists(ReturnStmt ret | this.getExpr() = ret.getExpr()) }
 
   /** Gets the kind of this returned value. */
-  ReturnKind getKind() { result = TNormalReturnKind() }
+  override ReturnKind getKind() { result = TNormalReturnKind() }
+}
+
+/**
+ * A `ReturnNode` that occurs as a result of a definition of a reference
+ * parameter reaching the end of a function body.
+ */
+private class RefReturnNode extends ReturnNode, RefParameterFinalValueNode {
+  /** Gets the kind of this returned value. */
+  override ReturnKind getKind() { result = TRefReturnKind(this.getParameter().getIndex()) }
 }
 
 /** A data flow node that represents the output of a call. */
-class OutNode extends ExprNode {
-  OutNode() { this.getExpr() instanceof Call }
+abstract class OutNode extends Node {
+  /** Gets the underlying call. */
+  abstract DataFlowCall getCall();
+}
+
+private class ExprOutNode extends OutNode, ExprNode {
+  ExprOutNode() { this.getExpr() instanceof Call }
 
   /** Gets the underlying call. */
-  DataFlowCall getCall() { result = this.getExpr() }
+  override DataFlowCall getCall() { result = this.getExpr() }
+}
+
+private class RefOutNode extends OutNode, DefinitionByReferenceNode {
+  /** Gets the underlying call. */
+  override DataFlowCall getCall() { result = this.getArgument().getParent() }
 }
 
 /**
@@ -85,6 +118,11 @@ class OutNode extends ExprNode {
 OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
   result = call.getNode() and
   kind = TNormalReturnKind()
+  or
+  exists(int i |
+    result.asDefiningArgument() = call.getArgument(i) and
+    kind = TRefReturnKind(i)
+  )
 }
 
 /**

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -25,7 +25,8 @@ private newtype TNode =
       not c.getTarget().getParameter(i).getUnderlyingType().(PointerType).getBaseType().isConst()
     )
   } or
-  TUninitializedNode(LocalVariable v) { not v.hasInitializer() }
+  TUninitializedNode(LocalVariable v) { not v.hasInitializer() } or
+  TRefParameterFinalValueNode(Parameter p) { exists(FlowVar var | var.reachesRefParameter(p)) }
 
 /**
  * A node in a data flow graph.
@@ -248,6 +249,23 @@ class UninitializedNode extends Node, TUninitializedNode {
   LocalVariable getLocalVariable() { result = v }
 }
 
+/** INTERNAL: do not use. The final value of a non-const ref parameter. */
+class RefParameterFinalValueNode extends Node, TRefParameterFinalValueNode {
+  Parameter p;
+
+  RefParameterFinalValueNode() { this = TRefParameterFinalValueNode(p) }
+
+  override Function getFunction() { result = p.getFunction() }
+
+  override Type getType() { result = p.getType() }
+
+  override string toString() { result = p.toString() }
+
+  override Location getLocation() { result = p.getLocation() }
+
+  Parameter getParameter() { result = p }
+}
+
 /**
  * A node associated with an object after an operation that might have
  * changed its state.
@@ -490,7 +508,7 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
       or
       var.definedPartiallyAt(nodeFrom.asPartialDefinition())
     ) and
-    varToExprStep(var, nodeTo.asExpr())
+    varToNodeStep(var, nodeTo)
   )
   or
   // Expr -> DefinitionByReferenceNode
@@ -533,9 +551,13 @@ private predicate exprToVarStep(Expr assignedExpr, FlowVar var) {
 }
 
 /**
- * Holds if the expression `e` is an access of the variable `var`.
+ * Holds if the node `n` is an access of the variable `var`.
  */
-private predicate varToExprStep(FlowVar var, Expr e) { e = var.getAnAccess() }
+private predicate varToNodeStep(FlowVar var, Node n) {
+  n.asExpr() = var.getAnAccess()
+  or
+  var.reachesRefParameter(n.(RefParameterFinalValueNode).getParameter())
+}
 
 /**
  * Holds if data flows from `fromExpr` to `toExpr` directly, in the case

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

@@ -62,9 +62,20 @@ class FlowVar extends TFlowVar {
   cached
   abstract predicate definedByReference(Expr arg);
 
+  /**
+   * Holds if this `FlowVar` is a `PartialDefinition` whose defined expression
+   * is `e`.
+   */
   cached
   abstract predicate definedPartiallyAt(Expr e);
 
+  /**
+   * Holds if this `FlowVar` is a definition of a reference parameter `p` that
+   * persists until the function returns.
+   */
+  cached
+  abstract predicate reachesRefParameter(Parameter p);
+
   /**
    * Holds if this `FlowVar` corresponds to the initial value of `v`. The following
    * is an exhaustive list of cases where this may happen.
@@ -338,6 +349,9 @@ module FlowVar_internal {
       param = v
     }
 
+    // `fullySupportedSsaVariable` excludes reference types
+    override predicate reachesRefParameter(Parameter p) { none() }
+
     /**
      * Holds if this `SsaVar` corresponds to a non-phi definition. Users of this
      * library will never directly use an `SsaVar` that comes from a phi node,
@@ -387,6 +401,13 @@ module FlowVar_internal {
       sbb = v.(Parameter).getFunction().getEntryPoint()
     }
 
+    override predicate reachesRefParameter(Parameter p) {
+      parameterIsNonConstReference(p) and
+      p = v and
+      // This definition reaches the exit node of the function CFG
+      getAReachedBlockVarSBB(this).getANode() = p.getFunction()
+    }
+
     override predicate definedByInitialValue(LocalScopeVariable lsv) {
       blockVarDefinedByVariable(sbb, lsv) and
       lsv = v
@@ -593,12 +614,23 @@ module FlowVar_internal {
   private predicate variableLiveInSBB(SubBasicBlock sbb, Variable v) {
     variableAccessInSBB(v, sbb, _)
     or
+    // Non-const reference parameters are live at the end of the function
+    parameterIsNonConstReference(v) and
+    sbb.contains(v.(Parameter).getFunction())
+    or
     exists(SubBasicBlock succ | succ = sbb.getASuccessor() |
       variableLiveInSBB(succ, v) and
       not variableNotLiveBefore(succ, v)
     )
   }
 
+  predicate parameterIsNonConstReference(Parameter p) {
+    exists(ReferenceType refType |
+      refType = p.getUnderlyingType() and
+      not refType.getBaseType().isConst()
+    )
+  }
+
   /**
    * Holds if liveness of `v` should stop propagating backwards from `sbb`.
    */