Swift: add ArrayContent summary support

rdmarsh2 · rdmarsh2 · commit d4b635d674df · 2023-07-17T19:09:05.000Z
diff --git a/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll b/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll
@@ -479,6 +479,9 @@ private predicate parseField(AccessPathToken c, Content::FieldContent f) {
 /** Holds if the specification component parses as a `Content`. */
 predicate parseContent(AccessPathToken component, Content content) {
   parseField(component, content)
+  or
+  component.getName() = "ArrayElement" and
+  content instanceof Content::ArrayContent
 }
 
 cached
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -8,7 +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.ArrayType
+private import codeql.swift.frameworks.StandardLibrary.Array
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(NodeImpl n) { result = n.getEnclosingCallable() }
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImplSpecific.qll b/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImplSpecific.qll
@@ -108,6 +108,10 @@ private string getContentSpecific(ContentSet cs) {
   exists(Content::FieldContent c |
     cs.isSingleton(c) and
     result = "Field[" + c.getField().getName() + "]"
+  ) or
+  exists(Content::ArrayContent c |
+    cs.isSingleton(c) and
+    result = "ArrayElement"
   )
 }
 
diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Array.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Array.qll
@@ -0,0 +1,18 @@
+import swift
+private import codeql.swift.dataflow.ExternalFlow
+
+class ArrayType extends BoundGenericType {
+  ArrayType() { this.getName().matches("Array<%") }
+}
+
+/**
+ * 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"
+    ]
+  }
+}
diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/ArrayType.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/ArrayType.qll
diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/StandardLibrary.qll b/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
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
@@ -292,6 +292,9 @@ edges
 | test.swift:647:15:647:15 | &... [Array element, Array element] | test.swift:647:15:647:23 | ...[...] [Array element] |
 | test.swift:647:15:647:23 | &... [Array element] | test.swift:647:15:647:26 | ...[...] |
 | test.swift:647:15:647:23 | ...[...] [Array element] | test.swift:647:15:647:23 | &... [Array element] |
+| test.swift:658:5:658:5 | [post] &... [Array element] | test.swift:659:15:659:15 | &... [Array element] |
+| test.swift:658:17:658:24 | call to source() | test.swift:658:5:658:5 | [post] &... [Array element] |
+| test.swift:659:15:659:15 | &... [Array element] | test.swift:659:15:659:21 | ...[...] |
 nodes
 | file://:0:0:0:0 | .a [x] | semmle.label | .a [x] |
 | file://:0:0:0:0 | .str | semmle.label | .str |
@@ -614,6 +617,10 @@ nodes
 | test.swift:647:15:647:23 | &... [Array element] | semmle.label | &... [Array element] |
 | test.swift:647:15:647:23 | ...[...] [Array element] | semmle.label | ...[...] [Array element] |
 | test.swift:647:15:647:26 | ...[...] | semmle.label | ...[...] |
+| test.swift:658:5:658:5 | [post] &... [Array element] | semmle.label | [post] &... [Array element] |
+| test.swift:658:17:658:24 | call to source() | semmle.label | call to source() |
+| test.swift:659:15:659:15 | &... [Array element] | semmle.label | &... [Array element] |
+| test.swift:659:15:659:21 | ...[...] | semmle.label | ...[...] |
 subpaths
 | test.swift:75:21:75:22 | &... | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:31:75:32 | [post] &... |
 | test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
@@ -733,3 +740,4 @@ subpaths
 | test.swift:639:15:639:21 | ...[...] | test.swift:638:15:638:22 | call to source() | test.swift:639:15:639:21 | ...[...] | result |
 | test.swift:643:15:643:21 | ...[...] | test.swift:642:17:642:24 | call to source() | test.swift:643:15:643:21 | ...[...] | result |
 | test.swift:647:15:647:26 | ...[...] | test.swift:645:20:645:27 | call to source() | test.swift:647:15:647:26 | ...[...] | result |
+| test.swift:659:15:659:21 | ...[...] | test.swift:658:17:658:24 | call to source() | test.swift:659:15:659:21 | ...[...] | result |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/FlowConfig.qll b/swift/ql/test/library-tests/dataflow/dataflow/FlowConfig.qll
@@ -5,6 +5,7 @@
 import swift
 import codeql.swift.dataflow.DataFlow
 import codeql.swift.dataflow.ExternalFlow
+import codeql.swift.frameworks.Frameworks
 
 module TestConfiguration implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node src) {
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
@@ -721,9 +721,11 @@
 | test.swift:654:9:654:9 | SSA def(arr5) | test.swift:655:15:655:15 | arr5 |
 | test.swift:654:9:654:9 | arr5 | test.swift:654:9:654:9 | SSA def(arr5) |
 | test.swift:654:16:654:51 | call to Array<Element>.init(repeating:count:) | test.swift:654:9:654:9 | arr5 |
+| test.swift:655:15:655:15 | arr5 | test.swift:655:15:655:15 | &... |
 | test.swift:657:9:657:9 | SSA def(arr6) | test.swift:658:5:658:5 | arr6 |
 | test.swift:657:9:657:9 | arr6 | test.swift:657:9:657:9 | SSA def(arr6) |
 | test.swift:657:16:657:22 | [...] | test.swift:657:9:657:9 | arr6 |
 | test.swift:658:5:658:5 | &... | test.swift:659:15:659:15 | arr6 |
 | test.swift:658:5:658:5 | [post] &... | test.swift:659:15:659:15 | arr6 |
 | test.swift:658:5:658:5 | arr6 | test.swift:658:5:658:5 | &... |
+| test.swift:659:15:659:15 | arr6 | test.swift:659:15:659:15 | &... |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/test.swift b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
@@ -652,9 +652,9 @@ func testArray() {
     sink(arg: arr4) // $ MISSING: flow=642
 
     var arr5 = Array(repeating: source(), count: 2)
-    sink(arg: arr5) // $ MISSING: flow=654
+    sink(arg: arr5[0]) // $ MISSING: flow=654
 
     var arr6 = [1,2,3]
     arr6.insert(source(), at: 2)
-    sink(arg: arr6) // $ MISSING: flow=658
+    sink(arg: arr6[0]) // $ flow=658
 }

Original file line number	Diff line number	Diff line change
`@@ -479,6 +479,9 @@ private predicate parseField(AccessPathToken c, Content::FieldContent f) {`
`479`	`479`	/** Holds if the specification component parses as a `Content`. */
`480`	`480`	`predicate parseContent(AccessPathToken component, Content content) {`
`481`	`481`	`parseField(component, content)`
	`482`	`+ or`
	`483`	`+ component.getName() = "ArrayElement" and`
	`484`	`+ content instanceof Content::ArrayContent`
`482`	`485`	`}`
`483`	`486`
`484`	`487`	`cached`
Original file line number	Diff line number	Diff line change
`@@ -108,6 +108,10 @@ private string getContentSpecific(ContentSet cs) {`
`108`	`108`	`exists(Content::FieldContent c \|`
`109`	`109`	`cs.isSingleton(c) and`
`110`	`110`	`result = "Field[" + c.getField().getName() + "]"`
	`111`	`+ ) or`
	`112`	`+ exists(Content::ArrayContent c \|`
	`113`	`+ cs.isSingleton(c) and`
	`114`	`+ result = "ArrayElement"`
`111`	`115`	`)`
`112`	`116`	`}`
`113`	`117`