Rust: Handle ref patterns in data flow

To do this we:
* Let SSA writes target the name inside identifier patterns instead of
  the pattern itself
* Include relevant names in the data flow graph
* Add a store step from a identifier patterns with `ref` into the
  contained name. So we have an edge `ref a` -> `a` that stores in the
  reference content type.

Author: paldepind

rust/ql/.generated.list

@@ -662,7 +662,7 @@ module PatternTrees {
     }
 
     override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      // Edge from succesful subpattern to name
+      // Edge from successful subpattern to name
       super.succ(pred, succ, c) and c.(MatchCompletion).succeeded()
       or
       // Edge from name to the identifier pattern itself

rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll

@@ -195,7 +195,7 @@ module Ssa {
       )
       or
       exists(LetStmtCfgNode ls |
-        ls.getPat() = write and
+        ls.getPat().(IdentPatCfgNode).getName() = write and
         ls.getInitializer() = value
       )
     }

rust/ql/lib/codeql/rust/controlflow/internal/generated/CfgNodes.qll

@@ -273,6 +273,15 @@ module Node {
     override PatCfgNode asPat() { result = n }
   }
 
+  /** A data flow node that corresponds to a name node in the CFG. */
+  final class NameNode extends AstCfgFlowNode, TNameNode {
+    override NameCfgNode n;
+
+    NameNode() { this = TNameNode(n) }
+
+    NameCfgNode asName() { result = n }
+  }
+
   /**
    * The value of a parameter at function entry, viewed as a node in a data
    * flow graph.
@@ -584,11 +593,23 @@ module LocalFlow {
   predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
     nodeFrom.getCfgNode() = getALastEvalNode(nodeTo.getCfgNode())
     or
+    // An edge from the right-hand side of a let statement to the left-hand side.
     exists(LetStmtCfgNode s |
       nodeFrom.getCfgNode() = s.getInitializer() and
       nodeTo.getCfgNode() = s.getPat()
     )
     or
+    exists(IdentPatCfgNode p |
+      not p.isRef() and
+      nodeFrom.getCfgNode() = p and
+      nodeTo.getCfgNode() = p.getName()
+    )
+    or
+    exists(SelfParamCfgNode self |
+      nodeFrom.getCfgNode() = self and
+      nodeTo.getCfgNode() = self.getName()
+    )
+    or
     // An edge from a pattern/expression to its corresponding SSA definition.
     nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
       nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()
@@ -1266,6 +1287,14 @@ module RustDataFlow implements InputSig<Location> {
         node2.asExpr().(ArrayListExprCfgNode).getAnExpr()
       ]
     or
+    // Store from a `ref` identifier pattern into the contained name.
+    exists(IdentPatCfgNode p |
+      c instanceof ReferenceContent and
+      p.isRef() and
+      node1.asPat() = p and
+      node2.(Node::NameNode).asName() = p.getName()
+    )
+    or
     fieldAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
     or
     referenceAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
@@ -1560,6 +1589,7 @@ private module Cached {
     TExprNode(ExprCfgNode n) { Stages::DataFlowStage::ref() } or
     TSourceParameterNode(ParamBaseCfgNode p) or
     TPatNode(PatCfgNode p) or
+    TNameNode(NameCfgNode n) { n.getName() = any(Variable v).getName() } or
     TExprPostUpdateNode(ExprCfgNode e) {
       isArgumentForCall(e, _, _) or
       lambdaCallExpr(_, _, e) or

rust/ql/lib/codeql/rust/dataflow/Ssa.qll

@@ -7,25 +7,25 @@ private import Cfg
 private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
 private import codeql.ssa.Ssa as SsaImplCommon
 
-/** Holds if `v` is introduced like `let v : i64;`. */
-private predicate isUnitializedLet(IdentPat pat, Variable v) {
-  pat = v.getPat() and
+/**
+ * Holds if `name` occurs in the left-hand side of an uninitialized let
+ * statement such as in `let name : i64;`.
+ */
+private predicate isInUninitializedLet(Name name) {
   exists(LetStmt let |
-    let = v.getLetStmt() and
+    let.getPat().(IdentPat).getName() = name and
     not let.hasInitializer()
   )
 }
 
 /** Holds if `write` writes to variable `v`. */
 predicate variableWrite(AstNode write, Variable v) {
-  exists(IdentPat pat |
-    pat = write and
-    pat = v.getPat() and
-    not isUnitializedLet(pat, v)
+  exists(Name name |
+    name = write and
+    name = v.getName() and
+    not isInUninitializedLet(name)
   )
   or
-  exists(SelfParam self | self = write and self = v.getSelfParam())
-  or
   exists(VariableAccess access |
     access = write and
     access.getVariable() = v

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -74,58 +74,66 @@ module Impl {
    * where `definingNode` is the entire `Either::Left(x) | Either::Right(x)`
    * pattern.
    */
-  private predicate variableDecl(AstNode definingNode, AstNode p, string name) {
-    p =
-      any(SelfParam sp |
-        definingNode = sp.getName() and
-        name = sp.getName().getText() and
+  private predicate variableDecl(AstNode definingNode, Name name, string text) {
+    text = name.getText() and
+    (
+      exists(SelfParam sp |
+        name = sp.getName() and
+        definingNode = name and
         // exclude self parameters from functions without a body as these are
         // trait method declarations without implementations
         not exists(Function f | not f.hasBody() and f.getParamList().getSelfParam() = sp)
       )
-    or
-    p =
-      any(IdentPat pat |
+      or
+      exists(IdentPat pat |
+        name = pat.getName() and
         (
           definingNode = getOutermostEnclosingOrPat(pat)
           or
-          not exists(getOutermostEnclosingOrPat(pat)) and definingNode = pat.getName()
+          not exists(getOutermostEnclosingOrPat(pat)) and definingNode = name
         ) and
-        name = pat.getName().getText() and
         // exclude for now anything starting with an uppercase character, which may be a reference to
         // an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
         // which we don't appear to recognize yet anyway. This also assumes programmers follow the
         // naming guidelines, which they generally do, but they're not enforced.
-        not name.charAt(0).isUppercase() and
+        not text.charAt(0).isUppercase() and
         // exclude parameters from functions without a body as these are trait method declarations
         // without implementations
         not exists(Function f | not f.hasBody() and f.getParamList().getAParam().getPat() = pat) and
         // exclude parameters from function pointer types (e.g. `x` in `fn(x: i32) -> i32`)
         not exists(FnPtrTypeRepr fp | fp.getParamList().getParam(_).getPat() = pat)
       )
+    )
   }
 
   /** A variable. */
   class Variable extends MkVariable {
     private AstNode definingNode;
-    private string name;
+    private string text;
 
-    Variable() { this = MkVariable(definingNode, name) }
+    Variable() { this = MkVariable(definingNode, text) }
 
-    /** Gets the name of this variable. */
-    string getName() { result = name }
+    /** Gets the name of this variable as a string. */
+    string getText() { result = text }
 
     /** Gets the location of this variable. */
     Location getLocation() { result = definingNode.getLocation() }
 
     /** Gets a textual representation of this variable. */
-    string toString() { result = this.getName() }
+    string toString() { result = this.getText() }
 
     /** Gets an access to this variable. */
     VariableAccess getAnAccess() { result.getVariable() = this }
 
-    /** Gets the `self` parameter that declares this variable, if one exists. */
-    SelfParam getSelfParam() { variableDecl(definingNode, result, name) }
+    /**
+     * Get the name of this variable.
+     *
+     * Normally, the name is unique, except when introduced in an or pattern.
+     */
+    Name getName() { variableDecl(definingNode, result, text) }
+
+    /** Gets the `self` parameter that declares this variable, if any. */
+    SelfParam getSelfParam() { result.getName() = this.getName() }
 
     /**
      * Gets the pattern that declares this variable, if any.
@@ -138,7 +146,7 @@ module Impl {
      * }
      * ```
      */
-    IdentPat getPat() { variableDecl(definingNode, result, name) }
+    IdentPat getPat() { result.getName() = this.getName() }
 
     /** Gets the enclosing CFG scope for this variable declaration. */
     CfgScope getEnclosingCfgScope() { result = definingNode.getEnclosingCfgScope() }
@@ -204,6 +212,10 @@ module Impl {
   /** Gets the immediately enclosing variable scope of `n`. */
   private VariableScope getEnclosingScope(AstNode n) { result = getAnAncestorInVariableScope(n) }
 
+  /**
+   * Get all the pattern ancestors of this variable up to an including the
+   * root of the pattern.
+   */
   private Pat getAVariablePatAncestor(Variable v) {
     result = v.getPat()
     or
@@ -322,7 +334,7 @@ module Impl {
    * all nodes nester under `scope`, is `ord`.
    */
   private predicate variableDeclInScope(Variable v, VariableScope scope, string name, int ord) {
-    name = v.getName() and
+    name = v.getText() and
     (
       parameterDeclInScope(v, scope) and
       ord = getPreOrderNumbering(scope, scope)