Rust: Improve handling of deref expressions in data flow

Author: hvitved

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

@@ -365,6 +365,7 @@ module RustDataFlow implements InputSig<Location> {
     node instanceof CaptureNode or
     node instanceof ClosureParameterNode or
     node instanceof DerefBorrowNode or
+    node instanceof DerefOutNode or
     node.asExpr() instanceof ParenExpr or
     nodeIsHidden(node.(PostUpdateNode).getPreUpdateNode())
   }
@@ -586,10 +587,9 @@ module RustDataFlow implements InputSig<Location> {
           .isVariantField([any(OptionEnum o).getSome(), any(ResultEnum r).getOk()], 0)
     )
     or
-    exists(PrefixExpr deref |
+    exists(DerefExpr deref |
       c instanceof ReferenceContent and
-      deref.getOperatorName() = "*" and
-      node1.asExpr() = deref.getExpr() and
+      node1.(DerefOutNode).getDerefExpr() = deref and
       node2.asExpr() = deref
     )
     or

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

@@ -348,15 +348,46 @@ abstract class OutNode extends Node {
 }
 
 final private class ExprOutNode extends ExprNode, OutNode {
-  ExprOutNode() { this.asExpr() instanceof Call }
+  ExprOutNode() {
+    exists(Call call |
+      call = this.asExpr() and
+      not call instanceof DerefExpr // Handled by `DerefOutNode`
+    )
+  }
 
-  /** Gets the underlying call CFG node that includes this out node. */
+  /** Gets the underlying call node that includes this out node. */
   override DataFlowCall getCall(ReturnKind kind) {
     result.asCall() = n and
     kind = TNormalReturnKind()
   }
 }
 
+/**
+ * A node that represents the value of a `*` expression _before_ implicit
+ * dereferencing:
+ *
+ * `*v` equivalent to `*Deref::deref(&v)`, and this node represents the
+ * `Deref::deref(&v)` part.
+ */
+class DerefOutNode extends OutNode, TDerefOutNode {
+  DerefExpr de;
+
+  DerefOutNode() { this = TDerefOutNode(de) }
+
+  DerefExpr getDerefExpr() { result = de }
+
+  override CfgScope getCfgScope() { result = de.getEnclosingCfgScope() }
+
+  override DataFlowCall getCall(ReturnKind kind) {
+    result.asCall() = de and
+    kind = TNormalReturnKind()
+  }
+
+  override Location getLocation() { result = de.getLocation() }
+
+  override string toString() { result = de.toString() + " [pre-dereferenced]" }
+}
+
 final class SummaryOutNode extends FlowSummaryNode, OutNode {
   private DataFlowCall call;
   private ReturnKind kind_;
@@ -499,6 +530,7 @@ newtype TNode =
     TypeInference::implicitBorrow(n) and
     borrow = true
   } or
+  TDerefOutNode(DerefExpr de) or
   TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
   TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) {
     forall(AstNode n | n = sn.getSinkElement() or n = sn.getSourceElement() |

rust/ql/lib/codeql/rust/frameworks/stdlib/Builtins.qll

@@ -27,8 +27,19 @@ private class BuiltinsTypesFile extends File {
 class BuiltinType extends Struct {
   BuiltinType() { this.getFile() instanceof BuiltinsTypesFile }
 
-  /** Gets the name of this type. */
+  /**
+   * Gets the name of this type.
+   *
+   * This is the name used internally to represent the type, for example `Ref`.
+   */
   string getName() { result = super.getName().getText() }
+
+  /**
+   * Gets a display name for this type.
+   *
+   * This is the name used in code, for example `&`.
+   */
+  string getDisplayName() { result = this.getName() }
 }
 
 /**
@@ -147,9 +158,18 @@ class ArrayType extends BuiltinType {
   ArrayType() { this.getName() = "Array" }
 }
 
-/** The builtin reference type `&T` or `&mut T`. */
+/** The builtin reference type `&T`. */
 class RefType extends BuiltinType {
   RefType() { this.getName() = "Ref" }
+
+  override string getDisplayName() { result = "&" }
+}
+
+/** The builtin reference type `&mut T`. */
+class RefMutType extends BuiltinType {
+  RefMutType() { this.getName() = "RefMut" }
+
+  override string getDisplayName() { result = "&mut" }
 }
 
 /** The builtin pointer type `*const T` or `*mut T`. */

rust/ql/lib/codeql/rust/frameworks/stdlib/alloc.model.yml

@@ -24,9 +24,10 @@ extensions:
       extensible: summaryModel
     data:
       # Box
-      - ["<alloc::boxed::Box>::pin", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
-      - ["<alloc::boxed::Box>::new", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
-      - ["<alloc::boxed::Box>::into_pin", "Argument[0]", "ReturnValue", "value", "manual"]
+      - ["<alloc::boxed::Box as core::ops::deref::Deref>::deref", "Argument[self].Field[alloc::boxed::Box(0)]", "ReturnValue.Reference", "value", "manual"]
+      - ["<alloc::boxed::Box>::pin", "Argument[0]", "ReturnValue.Field[core::pin::Pin::pointer].Field[alloc::boxed::Box(0)]", "value", "manual"]
+      - ["<alloc::boxed::Box>::new", "Argument[0]", "ReturnValue.Field[alloc::boxed::Box(0)]", "value", "manual"]
+      - ["<alloc::boxed::Box>::into_pin", "Argument[0]", "ReturnValue.Field[core::pin::Pin::pointer]", "value", "manual"]
       # Fmt
       - ["alloc::fmt::format", "Argument[0]", "ReturnValue", "taint", "manual"]
       # Layout

rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml

@@ -3,6 +3,9 @@ extensions:
       pack: codeql/rust-all
       extensible: summaryModel
     data:
+      # Builtin deref
+      - ["<& as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
+      - ["<&mut as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
       # Arithmetic
       - ["<_ as core::ops::arith::Add>::add", "Argument[self]", "ReturnValue", "taint", "manual"]
       - ["<_ as core::ops::arith::Add>::add", "Argument[0]", "ReturnValue", "taint", "manual"]
@@ -41,8 +44,9 @@ extensions:
       - ["core::ptr::write", "Argument[1]", "Argument[0].Reference", "value", "manual"]
       - ["core::ptr::write_unaligned", "Argument[1]", "Argument[0].Reference", "value", "manual"]
       - ["core::ptr::write_volatile", "Argument[1]", "Argument[0].Reference", "value", "manual"]
-        # https://doc.rust-lang.org/std/pin/struct.Pin.html#impl-Deref-for-Pin%3CPtr%3E, but limited to `Ptr = &`
+        # https://doc.rust-lang.org/std/pin/struct.Pin.html#impl-Deref-for-Pin%3CPtr%3E, but limited to `Ptr = &` and `Ptr = Box`
       - ["<core::pin::Pin as core::ops::deref::Deref>::deref", "Argument[self].Reference.Field[core::pin::Pin::pointer].Reference", "ReturnValue.Reference", "value", "manual"]
+      - ["<core::pin::Pin as core::ops::deref::Deref>::deref", "Argument[self].Reference.Field[core::pin::Pin::pointer].Field[alloc::boxed::Box(0)]", "ReturnValue.Reference", "value", "manual"]
       # Str
       - ["<core::str>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
       - ["<core::str>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]

rust/ql/lib/codeql/rust/internal/PathResolution.qll

@@ -958,7 +958,16 @@ private class StructItemNode extends TypeItemNode, ParameterizableItemNode insta
   language[monotonicAggregates]
   override string getCanonicalPath(Crate c) {
     this.hasCanonicalPath(c) and
-    result = strictconcat(int i | i in [0 .. 2] | this.getCanonicalPathPart(c, i) order by i)
+    (
+      this =
+        any(Builtins::BuiltinType t |
+          not t.hasVisibility() and
+          result = t.getDisplayName()
+        )
+      or
+      not this = any(Builtins::BuiltinType t | not t.hasVisibility()) and
+      result = strictconcat(int i | i in [0 .. 2] | this.getCanonicalPathPart(c, i) order by i)
+    )
   }
 }