Swift: add DataFlow::Content for arrays

swift/ql/lib/change-notes/2023-07-18-array-content.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* Added `DataFlow::ArrayContent`, which will provide more accurate flow through arrays.

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

@@ -1235,14 +1235,6 @@ module Exprs {
     }
   }
 
-  private class InOutTree extends AstStandardPostOrderTree {
-    override InOutExpr ast;
-
-    final override ControlFlowElement getChildElement(int i) {
-      i = 0 and result.asAstNode() = ast.getSubExpr().getFullyConverted()
-    }
-  }
-
   private class SubscriptTree extends AstControlFlowTree {
     override SubscriptExpr ast;
 
@@ -1749,7 +1741,8 @@ module Exprs {
 
   module Conversions {
     class ConversionOrIdentity =
-      Synth::TIdentityExpr or Synth::TExplicitCastExpr or Synth::TImplicitConversionExpr;
+      Synth::TIdentityExpr or Synth::TExplicitCastExpr or Synth::TImplicitConversionExpr or
+          Synth::TInOutExpr;
 
     abstract class ConversionOrIdentityTree extends AstStandardPostOrderTree {
       ConversionOrIdentityTree() { ast instanceof ConversionOrIdentity }
@@ -1780,6 +1773,12 @@ module Exprs {
 
       override predicate convertsFrom(Expr e) { ast.convertsFrom(e) }
     }
+
+    private class InOutTree extends ConversionOrIdentityTree {
+      override InOutExpr ast;
+
+      override predicate convertsFrom(Expr e) { ast.convertsFrom(e) }
+    }
   }
 }
 

swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll

@@ -486,8 +486,12 @@ private predicate parseEnum(AccessPathToken c, Content::EnumContent f) {
 
 /** Holds if the specification component parses as a `Content`. */
 predicate parseContent(AccessPathToken component, Content content) {
-  parseField(component, content) or
+  parseField(component, content)
+  or
   parseEnum(component, content)
+  or
+  component.getName() = "ArrayElement" and
+  content instanceof Content::ArrayContent
 }
 
 cached

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

@@ -8,6 +8,7 @@ private import codeql.swift.controlflow.BasicBlocks
 private import codeql.swift.dataflow.FlowSummary as FlowSummary
 private import codeql.swift.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import codeql.swift.frameworks.StandardLibrary.PointerTypes
+private import codeql.swift.frameworks.StandardLibrary.Array
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(NodeImpl n) { result = n.getEnclosingCallable() }
@@ -110,7 +111,8 @@ private module Cached {
       hasExprNode(n,
         [
           any(Argument arg | modifiable(arg)).getExpr(), any(MemberRefExpr ref).getBase(),
-          any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr()
+          any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr(),
+          any(SubscriptExpr se).getBase()
         ])
     }
 
@@ -166,6 +168,10 @@ private module Cached {
     // flow through `&` (inout argument)
     nodeFrom.asExpr() = nodeTo.asExpr().(InOutExpr).getSubExpr()
     or
+    // reverse flow through `&` (inout argument)
+    nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr().(InOutExpr).getSubExpr() =
+      nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr()
+    or
     // flow through `try!` and similar constructs
     nodeFrom.asExpr() = nodeTo.asExpr().(AnyTryExpr).getSubExpr()
     or
@@ -249,7 +255,8 @@ private module Cached {
   newtype TContent =
     TFieldContent(FieldDecl f) or
     TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
-    TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) }
+    TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) } or
+    TArrayContent()
 }
 
 /**
@@ -695,6 +702,22 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
     init.isFailable()
   )
   or
+  // creation of an array `[v1,v2]`
+  exists(ArrayExpr arr |
+    node1.asExpr() = arr.getAnElement() and
+    node2.asExpr() = arr and
+    c.isSingleton(any(Content::ArrayContent ac))
+  )
+  or
+  // array assignment `a[n] = x`
+  exists(AssignExpr assign, SubscriptExpr subscript |
+    node1.asExpr() = assign.getSource() and
+    node2.(PostUpdateNode).getPreUpdateNode().asExpr() = subscript.getBase() and
+    subscript = assign.getDest() and
+    subscript.getBase().getType() instanceof ArrayType and
+    c.isSingleton(any(Content::ArrayContent ac))
+  )
+  or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
 }
@@ -750,10 +773,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
   )
   or
   // read of a component in a key-path expression chain
-  exists(KeyPathComponent component, FieldDecl f |
+  exists(KeyPathComponent component |
     component = node1.(KeyPathComponentNodeImpl).getComponent() and
-    f = component.getDeclRef() and
-    c.isSingleton(any(Content::FieldContent ct | ct.getField() = f))
+    (
+      c.isSingleton(any(Content::FieldContent ct | ct.getField() = component.getDeclRef()))
+      or
+      c.isSingleton(any(Content::ArrayContent ac)) and
+      component.isSubscript()
+    )
   |
     // the next node is either the next element in the chain
     node2.(KeyPathComponentNodeImpl).getComponent() = component.getNextComponent()
@@ -762,6 +789,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
     not exists(component.getNextComponent()) and
     node2.(KeyPathReturnNodeImpl).getKeyPathExpr() = component.getKeyPathExpr()
   )
+  or
+  // read of an array member via subscript operator
+  exists(SubscriptExpr subscript |
+    subscript.getBase() = node1.asExpr() and
+    subscript = node2.asExpr() and
+    subscript.getBase().getType() instanceof ArrayType and
+    c.isSingleton(any(Content::ArrayContent ac))
+  )
 }
 
 /**
@@ -873,7 +908,7 @@ int accessPathLimit() { result = 5 }
  * Holds if access paths with `c` at their head always should be tracked at high
  * precision. This disables adaptive access path precision for such access paths.
  */
-predicate forceHighPrecision(Content c) { none() }
+predicate forceHighPrecision(Content c) { c instanceof Content::ArrayContent }
 
 /**
  * Holds if the node `n` is unreachable when the call context is `call`.

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll

@@ -219,6 +219,11 @@ module Content {
 
     override string toString() { result = this.getSignature() }
   }
+
+  /** An element of an array at an unknown index */
+  class ArrayContent extends Content, TArrayContent {
+    override string toString() { result = "Array element" }
+  }
 }
 
 /**

swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -109,6 +109,11 @@ private string getContentSpecific(ContentSet cs) {
     cs.isSingleton(c) and
     result = "Field[" + c.getField().getName() + "]"
   )
+  or
+  exists(Content::ArrayContent c |
+    cs.isSingleton(c) and
+    result = "ArrayElement"
+  )
 }
 
 /** Gets the textual representation of a summary component in the format used for MaD models. */

swift/ql/lib/codeql/swift/elements/expr/InOutExpr.qll

@@ -2,4 +2,6 @@ private import codeql.swift.generated.expr.InOutExpr
 
 class InOutExpr extends Generated::InOutExpr {
   override string toString() { result = "&..." }
+
+  override predicate convertsFrom(Expr e) { e = this.getImmediateSubExpr() }
 }

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Array.qll

@@ -0,0 +1,26 @@
+/**
+ * Provides models for `Array` and related Swift classes.
+ */
+
+import swift
+private import codeql.swift.dataflow.ExternalFlow
+
+/**
+ * An instance of the `Array` type.
+ */
+class ArrayType extends Type {
+  ArrayType() { this.getName().matches("Array<%") or this.getName().matches("[%]") }
+}
+
+/**
+ * A model for `Array` and related class members that permit data flow.
+ */
+private class ArraySummaries extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";Array;true;insert(_:at:);;;Argument[0];Argument[-1].ArrayElement;value",
+        ";Array;true;insert(_:at:);;;Argument[1];Argument[-1];taint"
+      ]
+  }
+}

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/StandardLibrary.qll

@@ -2,6 +2,7 @@
  * This file imports all models related to the Swift standard library.
  */
 
+private import Array
 private import Collection
 private import CustomUrlSchemes
 private import Data

swift/ql/test/extractor-tests/generated/decl/CapturedDecl/PrintAst.expected

@@ -72,9 +72,9 @@ closures.swift:
 #   17|               getBase(): [TypeExpr] Int.Type
 #   17|                 getTypeRepr(): [TypeRepr] Int
 #   17|               getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   17|             getArgument(0): [Argument] : &...
-#   17|               getExpr(): [InOutExpr] &...
-#   17|                 getSubExpr(): [DeclRefExpr] x
+#   17|             getArgument(0): [Argument] : x
+#   17|               getExpr(): [DeclRefExpr] x
+#   17|               getExpr().getFullyConverted(): [InOutExpr] &...
 #   17|             getArgument(1): [Argument] : 1
 #   17|               getExpr(): [IntegerLiteralExpr] 1
 #   18|           getElement(1): [CallExpr] call to print(_:separator:terminator:)
@@ -292,9 +292,9 @@ closures.swift:
 #   52|                 getBase(): [TypeExpr] Int.Type
 #   52|                   getTypeRepr(): [TypeRepr] Int
 #   52|                 getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   52|               getArgument(0): [Argument] : &...
-#   52|                 getExpr(): [InOutExpr] &...
-#   52|                   getSubExpr(): [DeclRefExpr] x
+#   52|               getArgument(0): [Argument] : x
+#   52|                 getExpr(): [DeclRefExpr] x
+#   52|                 getExpr().getFullyConverted(): [InOutExpr] &...
 #   52|               getArgument(1): [Argument] : y
 #   52|                 getExpr(): [DeclRefExpr] y
 #   52|         getCapture(0): [CapturedDecl] x
@@ -304,9 +304,9 @@ closures.swift:
 #   53|         getBase(): [TypeExpr] Int.Type
 #   53|           getTypeRepr(): [TypeRepr] Int
 #   53|         getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   53|       getArgument(0): [Argument] : &...
-#   53|         getExpr(): [InOutExpr] &...
-#   53|           getSubExpr(): [DeclRefExpr] x
+#   53|       getArgument(0): [Argument] : x
+#   53|         getExpr(): [DeclRefExpr] x
+#   53|         getExpr().getFullyConverted(): [InOutExpr] &...
 #   53|       getArgument(1): [Argument] : 40
 #   53|         getExpr(): [IntegerLiteralExpr] 40
 #   54|     getElement(3): [PatternBindingDecl] var ... = ...
@@ -351,9 +351,9 @@ closures.swift:
 #   61|                 getBase(): [TypeExpr] Int.Type
 #   61|                   getTypeRepr(): [TypeRepr] Int
 #   61|                 getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   61|               getArgument(0): [Argument] : &...
-#   61|                 getExpr(): [InOutExpr] &...
-#   61|                   getSubExpr(): [DeclRefExpr] x
+#   61|               getArgument(0): [Argument] : x
+#   61|                 getExpr(): [DeclRefExpr] x
+#   61|                 getExpr().getFullyConverted(): [InOutExpr] &...
 #   61|               getArgument(1): [Argument] : y
 #   61|                 getExpr(): [DeclRefExpr] y
 #   61|         getCapture(0): [CapturedDecl] x
@@ -363,9 +363,9 @@ closures.swift:
 #   62|         getBase(): [TypeExpr] Int.Type
 #   62|           getTypeRepr(): [TypeRepr] Int
 #   62|         getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   62|       getArgument(0): [Argument] : &...
-#   62|         getExpr(): [InOutExpr] &...
-#   62|           getSubExpr(): [DeclRefExpr] x
+#   62|       getArgument(0): [Argument] : x
+#   62|         getExpr(): [DeclRefExpr] x
+#   62|         getExpr().getFullyConverted(): [InOutExpr] &...
 #   62|       getArgument(1): [Argument] : 40
 #   62|         getExpr(): [IntegerLiteralExpr] 40
 #   63|     getElement(3): [PatternBindingDecl] var ... = ...
@@ -417,9 +417,9 @@ closures.swift:
 #   71|                 getBase(): [TypeExpr] Int.Type
 #   71|                   getTypeRepr(): [TypeRepr] Int
 #   71|                 getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   71|               getArgument(0): [Argument] : &...
-#   71|                 getExpr(): [InOutExpr] &...
-#   71|                   getSubExpr(): [DeclRefExpr] x
+#   71|               getArgument(0): [Argument] : x
+#   71|                 getExpr(): [DeclRefExpr] x
+#   71|                 getExpr().getFullyConverted(): [InOutExpr] &...
 #   71|               getArgument(1): [Argument] : y
 #   71|                 getExpr(): [DeclRefExpr] y
 #   71|         getCapture(0): [CapturedDecl] x
@@ -429,9 +429,9 @@ closures.swift:
 #   72|         getBase(): [TypeExpr] Int.Type
 #   72|           getTypeRepr(): [TypeRepr] Int
 #   72|         getMethodRef(): [DeclRefExpr] +=(_:_:)
-#   72|       getArgument(0): [Argument] : &...
-#   72|         getExpr(): [InOutExpr] &...
-#   72|           getSubExpr(): [DeclRefExpr] x
+#   72|       getArgument(0): [Argument] : x
+#   72|         getExpr(): [DeclRefExpr] x
+#   72|         getExpr().getFullyConverted(): [InOutExpr] &...
 #   72|       getArgument(1): [Argument] : 40
 #   72|         getExpr(): [IntegerLiteralExpr] 40
 #   73|     getElement(3): [PatternBindingDecl] var ... = ...
@@ -502,9 +502,9 @@ closures.swift:
 #   86|                 getBase(): [TypeExpr] Int.Type
 #   86|                   getTypeRepr(): [TypeRepr] Int
 #   86|                 getMethodRef(): [DeclRefExpr] -=(_:_:)
-#   86|               getArgument(0): [Argument] : &...
-#   86|                 getExpr(): [InOutExpr] &...
-#   86|                   getSubExpr(): [DeclRefExpr] x
+#   86|               getArgument(0): [Argument] : x
+#   86|                 getExpr(): [DeclRefExpr] x
+#   86|                 getExpr().getFullyConverted(): [InOutExpr] &...
 #   86|               getArgument(1): [Argument] : 1
 #   86|                 getExpr(): [IntegerLiteralExpr] 1
 #   87|             getElement(2): [IfStmt] if ... then { ... }
@@ -563,9 +563,9 @@ closures.swift:
 #   94|                 getBase(): [TypeExpr] Int.Type
 #   94|                   getTypeRepr(): [TypeRepr] Int
 #   94|                 getMethodRef(): [DeclRefExpr] -=(_:_:)
-#   94|               getArgument(0): [Argument] : &...
-#   94|                 getExpr(): [InOutExpr] &...
-#   94|                   getSubExpr(): [DeclRefExpr] x
+#   94|               getArgument(0): [Argument] : x
+#   94|                 getExpr(): [DeclRefExpr] x
+#   94|                 getExpr().getFullyConverted(): [InOutExpr] &...
 #   94|               getArgument(1): [Argument] : 1
 #   94|                 getExpr(): [IntegerLiteralExpr] 1
 #   95|             getElement(2): [IfStmt] if ... then { ... }
@@ -629,9 +629,9 @@ closures.swift:
 #  111|                           getBase(): [TypeExpr] Int.Type
 #  111|                             getTypeRepr(): [TypeRepr] Int
 #  111|                           getMethodRef(): [DeclRefExpr] +=(_:_:)
-#  111|                         getArgument(0): [Argument] : &...
-#  111|                           getExpr(): [InOutExpr] &...
-#  111|                             getSubExpr(): [DeclRefExpr] x
+#  111|                         getArgument(0): [Argument] : x
+#  111|                           getExpr(): [DeclRefExpr] x
+#  111|                           getExpr().getFullyConverted(): [InOutExpr] &...
 #  111|                         getArgument(1): [Argument] : 1
 #  111|                           getExpr(): [IntegerLiteralExpr] 1
 #  111|                   getCapture(0): [CapturedDecl] x

swift/ql/test/extractor-tests/statements/YieldStmts.expected

@@ -1,2 +1,2 @@
-| statements.swift:75:7:75:7 | yield ... | 0 | file://:0:0:0:0 | &... |
-| statements.swift:78:7:78:14 | yield ... | 0 | statements.swift:78:13:78:14 | &... |
+| statements.swift:75:7:75:7 | yield ... | 0 | file://:0:0:0:0 | .x |
+| statements.swift:78:7:78:14 | yield ... | 0 | statements.swift:78:14:78:14 | .x |