Swift: Improvements related to the swift/cleartext-logging query.

swift/ql/lib/change-notes/2023-08-16-varargs.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added flow through variadic arguments, and the `getVaList` function.

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

@@ -221,6 +221,9 @@ private module Cached {
     // flow through unary `+` (which does nothing)
     nodeFrom.asExpr() = nodeTo.asExpr().(UnaryPlusExpr).getOperand()
     or
+    // flow through varargs expansions (that wrap an `ArrayExpr` where varargs enter a call)
+    nodeFrom.asExpr() = nodeTo.asExpr().(VarargExpansionExpr).getSubExpr()
+    or
     // flow through nil-coalescing operator `??`
     exists(BinaryExpr nco |
       nco.getOperator().(FreeFunction).getName() = "??(_:_:)" and
@@ -966,7 +969,7 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
     node2.(KeyPathReturnNodeImpl).getKeyPathExpr() = component.getKeyPathExpr()
   )
   or
-  // read of an array member via subscript operator
+  // read of array or collection content via subscript operator
   exists(SubscriptExpr subscript |
     subscript.getBase() = node1.asExpr() and
     subscript = node2.asExpr() and

swift/ql/lib/codeql/swift/elements/type/VariadicSequenceType.qll

@@ -1,4 +1,12 @@
-// generated by codegen/codegen.py, remove this comment if you wish to edit this file
 private import codeql.swift.generated.type.VariadicSequenceType
 
+/**
+ * A variadic sequence type, that is, an array-like type that holds
+ * variadic arguments. For example the type `Int...` of `args` in:
+ * ```
+ * func myVarargsFunction(args: Int...) {
+ *   ...
+ * }
+ * ```
+ */
 class VariadicSequenceType extends Generated::VariadicSequenceType { }

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

@@ -0,0 +1,12 @@
+/**
+ * Provides models for Swift "C Interoperability" functions.
+ */
+
+import swift
+private import codeql.swift.dataflow.ExternalFlow
+
+private class CInteropSummaries extends SummaryModelCsv {
+  override predicate row(string row) {
+    row = ";;false;getVaList(_:);;;Argument[0].ArrayElement;ReturnValue;value"
+  }
+}

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

@@ -3,6 +3,7 @@
  */
 
 private import Array
+private import CInterop
 private import Collection
 private import CustomUrlSchemes
 private import Data

swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll

@@ -97,14 +97,17 @@ private class LoggingSinks extends SinkModelCsv {
   override predicate row(string row) {
     row =
       [
-        ";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;log-injection",
-        ";;false;print(_:separator:terminator:);;;Argument[1..2];log-injection",
-        ";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;log-injection",
-        ";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];log-injection",
-        ";;false;NSLog(_:_:);;;Argument[0];log-injection",
-        ";;false;NSLog(_:_:);;;Argument[1].ArrayElement;log-injection",
-        ";;false;NSLogv(_:_:);;;Argument[0];log-injection",
-        ";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;log-injection",
+        ";;false;print(_:separator:terminator:);;;Argument[0..2];log-injection",
+        ";;false;print(_:separator:terminator:toStream:);;;Argument[0..2];log-injection",
+        ";;false;print(_:separator:terminator:to:);;;Argument[0..2];log-injection",
+        ";;false;debugPrint(_:separator:terminator:);;;Argument[0..2];log-injection",
+        ";;false;debugPrint(_:separator:terminator:to:);;;Argument[0..2];log-injection",
+        ";;false;dump(_:name:indent:maxDepth:maxItems:);;;Argument[0..1];log-injection",
+        ";;false;dump(_:to:name:indent:maxDepth:maxItems:);;;Argument[0];log-injection",
+        ";;false;dump(_:to:name:indent:maxDepth:maxItems:);;;Argument[2];log-injection",
+        ";;false;fatalError(_:file:line:);;;Argument[0];log-injection",
+        ";;false;NSLog(_:_:);;;Argument[0..1];log-injection",
+        ";;false;NSLogv(_:_:);;;Argument[0..1];log-injection",
         ";;false;vfprintf(_:_:_:);;;Agument[1..2];log-injection",
         ";Logger;true;log(_:);;;Argument[0];log-injection",
         ";Logger;true;log(level:_:);;;Argument[1];log-injection",

swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll

@@ -25,6 +25,12 @@ module CleartextLoggingConfig implements DataFlow::ConfigSig {
   predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     any(CleartextLoggingAdditionalFlowStep s).step(n1, n2)
   }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
+    // flow out from collection content at the sink.
+    isSink(node) and
+    c.getAReadContent() instanceof DataFlow::Content::CollectionContent
+  }
 }
 
 /**

swift/ql/src/change-notes/2023-08-16-cleartext-logging.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added new logging sinks to the `swift/cleartext-logging` query.

swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected

@@ -519,6 +519,25 @@ edges
 | test.swift:841:13:841:13 | s2 [s, x] | test.swift:841:13:841:16 | .s [x] |
 | test.swift:841:13:841:16 | .s [x] | test.swift:615:7:615:7 | self [x] |
 | test.swift:841:13:841:16 | .s [x] | test.swift:841:13:841:18 | .x |
+| test.swift:844:19:844:28 | args [Collection element] | test.swift:846:15:846:15 | args [Collection element] |
+| test.swift:846:15:846:15 | args [Collection element] | test.swift:846:15:846:21 | ...[...] |
+| test.swift:849:19:849:24 | v | test.swift:850:15:850:15 | v |
+| test.swift:856:29:856:40 | args [Collection element] | test.swift:859:15:859:15 | args [Collection element] |
+| test.swift:856:29:856:40 | args [Collection element] | test.swift:860:15:860:15 | args [Collection element] |
+| test.swift:856:29:856:40 | args [Collection element] | test.swift:867:15:867:15 | args [Collection element] |
+| test.swift:859:15:859:15 | args [Collection element] | test.swift:859:15:859:21 | ...[...] |
+| test.swift:860:15:860:15 | args [Collection element] | test.swift:860:15:860:21 | ...[...] |
+| test.swift:866:21:866:29 | enter #keyPath(...) [Collection element] | test.swift:866:27:866:29 | KeyPathComponent [Collection element] |
+| test.swift:866:27:866:29 | KeyPathComponent [Collection element] | test.swift:866:21:866:29 | exit #keyPath(...) |
+| test.swift:867:15:867:15 | args [Collection element] | test.swift:866:21:866:29 | enter #keyPath(...) [Collection element] |
+| test.swift:867:15:867:15 | args [Collection element] | test.swift:867:15:867:38 | \\...[...] |
+| test.swift:871:24:871:31 | [...] [Collection element] | test.swift:844:19:844:28 | args [Collection element] |
+| test.swift:871:24:871:31 | [...] [Collection element] | test.swift:871:24:871:31 | [...] [Collection element] |
+| test.swift:871:24:871:31 | call to source() | test.swift:871:24:871:31 | [...] [Collection element] |
+| test.swift:872:18:872:25 | call to source() | test.swift:849:19:849:24 | v |
+| test.swift:873:21:873:31 | [...] [Collection element] | test.swift:856:29:856:40 | args [Collection element] |
+| test.swift:873:21:873:31 | [...] [Collection element] | test.swift:873:21:873:31 | [...] [Collection element] |
+| test.swift:873:24:873:31 | call to source() | test.swift:873:21:873:31 | [...] [Collection element] |
 nodes
 | file://:0:0:0:0 | .a [x] | semmle.label | .a [x] |
 | file://:0:0:0:0 | .s [x] | semmle.label | .s [x] |
@@ -1082,6 +1101,28 @@ nodes
 | test.swift:841:13:841:13 | s2 [s, x] | semmle.label | s2 [s, x] |
 | test.swift:841:13:841:16 | .s [x] | semmle.label | .s [x] |
 | test.swift:841:13:841:18 | .x | semmle.label | .x |
+| test.swift:844:19:844:28 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:846:15:846:15 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:846:15:846:21 | ...[...] | semmle.label | ...[...] |
+| test.swift:849:19:849:24 | v | semmle.label | v |
+| test.swift:850:15:850:15 | v | semmle.label | v |
+| test.swift:856:29:856:40 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:859:15:859:15 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:859:15:859:21 | ...[...] | semmle.label | ...[...] |
+| test.swift:860:15:860:15 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:860:15:860:21 | ...[...] | semmle.label | ...[...] |
+| test.swift:866:21:866:29 | enter #keyPath(...) [Collection element] | semmle.label | enter #keyPath(...) [Collection element] |
+| test.swift:866:21:866:29 | exit #keyPath(...) | semmle.label | exit #keyPath(...) |
+| test.swift:866:27:866:29 | KeyPathComponent [Collection element] | semmle.label | KeyPathComponent [Collection element] |
+| test.swift:867:15:867:15 | args [Collection element] | semmle.label | args [Collection element] |
+| test.swift:867:15:867:38 | \\...[...] | semmle.label | \\...[...] |
+| test.swift:871:24:871:31 | [...] [Collection element] | semmle.label | [...] [Collection element] |
+| test.swift:871:24:871:31 | [...] [Collection element] | semmle.label | [...] [Collection element] |
+| test.swift:871:24:871:31 | call to source() | semmle.label | call to source() |
+| test.swift:872:18:872:25 | call to source() | semmle.label | call to source() |
+| test.swift:873:21:873:31 | [...] [Collection element] | semmle.label | [...] [Collection element] |
+| test.swift:873:21:873:31 | [...] [Collection element] | semmle.label | [...] [Collection element] |
+| test.swift:873:24:873:31 | call to source() | semmle.label | call to source() |
 subpaths
 | test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
 | 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 ... |
@@ -1147,6 +1188,7 @@ subpaths
 | test.swift:840:3:840:16 | \\...[...] | test.swift:839:11:839:17 | [post] exit #keyPath(...) | test.swift:839:15:839:15 | [post] KeyPathComponent [s, x] | test.swift:840:3:840:3 | [post] s2 [s, x] |
 | test.swift:841:13:841:13 | s2 [s, x] | test.swift:632:7:632:7 | self [s, x] | file://:0:0:0:0 | .s [x] | test.swift:841:13:841:16 | .s [x] |
 | test.swift:841:13:841:16 | .s [x] | test.swift:615:7:615:7 | self [x] | file://:0:0:0:0 | .x | test.swift:841:13:841:18 | .x |
+| test.swift:867:15:867:15 | args [Collection element] | test.swift:866:21:866:29 | enter #keyPath(...) [Collection element] | test.swift:866:21:866:29 | exit #keyPath(...) | test.swift:867:15:867:38 | \\...[...] |
 #select
 | test.swift:7:15:7:15 | t1 | test.swift:6:19:6:26 | call to source() | test.swift:7:15:7:15 | t1 | result |
 | test.swift:9:15:9:15 | t1 | test.swift:6:19:6:26 | call to source() | test.swift:9:15:9:15 | t1 | result |
@@ -1262,3 +1304,8 @@ subpaths
 | test.swift:831:15:831:18 | .v | test.swift:828:12:828:19 | call to source() | test.swift:831:15:831:18 | .v | result |
 | test.swift:833:15:833:23 | call to getv() | test.swift:828:12:828:19 | call to source() | test.swift:833:15:833:23 | call to getv() | result |
 | test.swift:841:13:841:18 | .x | test.swift:840:20:840:27 | call to source() | test.swift:841:13:841:18 | .x | result |
+| test.swift:846:15:846:21 | ...[...] | test.swift:871:24:871:31 | call to source() | test.swift:846:15:846:21 | ...[...] | result |
+| test.swift:850:15:850:15 | v | test.swift:872:18:872:25 | call to source() | test.swift:850:15:850:15 | v | result |
+| test.swift:859:15:859:21 | ...[...] | test.swift:873:24:873:31 | call to source() | test.swift:859:15:859:21 | ...[...] | result |
+| test.swift:860:15:860:21 | ...[...] | test.swift:873:24:873:31 | call to source() | test.swift:860:15:860:21 | ...[...] | result |
+| test.swift:867:15:867:38 | \\...[...] | test.swift:873:24:873:31 | call to source() | test.swift:867:15:867:38 | \\...[...] | result |

swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected

@@ -1045,3 +1045,34 @@
 | test.swift:840:3:840:3 | [post] s2 | test.swift:841:13:841:13 | s2 |
 | test.swift:840:3:840:3 | s2 | test.swift:841:13:841:13 | s2 |
 | test.swift:840:20:840:27 | call to source() | test.swift:840:3:840:16 | \\...[...] |
+| test.swift:844:19:844:28 | SSA def(args) | test.swift:845:15:845:15 | args |
+| test.swift:844:19:844:28 | args | test.swift:844:19:844:28 | SSA def(args) |
+| test.swift:845:15:845:15 | args | test.swift:846:15:846:15 | args |
+| test.swift:849:19:849:24 | SSA def(v) | test.swift:850:15:850:15 | v |
+| test.swift:849:19:849:24 | v | test.swift:849:19:849:24 | SSA def(v) |
+| test.swift:849:29:849:40 | SSA def(args) | test.swift:851:15:851:15 | args |
+| test.swift:849:29:849:40 | args | test.swift:849:29:849:40 | SSA def(args) |
+| test.swift:851:15:851:15 | args | test.swift:852:15:852:15 | args |
+| test.swift:852:15:852:15 | [post] args | test.swift:853:15:853:15 | args |
+| test.swift:852:15:852:15 | args | test.swift:853:15:853:15 | args |
+| test.swift:856:19:856:24 | SSA def(v) | test.swift:857:15:857:15 | v |
+| test.swift:856:19:856:24 | v | test.swift:856:19:856:24 | SSA def(v) |
+| test.swift:856:29:856:40 | SSA def(args) | test.swift:858:15:858:15 | args |
+| test.swift:856:29:856:40 | args | test.swift:856:29:856:40 | SSA def(args) |
+| test.swift:858:15:858:15 | args | test.swift:859:15:859:15 | args |
+| test.swift:859:15:859:15 | [post] args | test.swift:860:15:860:15 | args |
+| test.swift:859:15:859:15 | args | test.swift:860:15:860:15 | args |
+| test.swift:860:15:860:15 | [post] args | test.swift:862:16:862:16 | args |
+| test.swift:860:15:860:15 | args | test.swift:862:16:862:16 | args |
+| test.swift:862:9:862:9 | SSA def(arg) | test.swift:863:19:863:19 | arg |
+| test.swift:862:9:862:9 | arg | test.swift:862:9:862:9 | SSA def(arg) |
+| test.swift:862:16:862:16 | [post] args | test.swift:867:15:867:15 | args |
+| test.swift:862:16:862:16 | args | test.swift:867:15:867:15 | args |
+| test.swift:866:9:866:9 | SSA def(myKeyPath) | test.swift:867:29:867:29 | myKeyPath |
+| test.swift:866:9:866:9 | myKeyPath | test.swift:866:9:866:9 | SSA def(myKeyPath) |
+| test.swift:866:21:866:29 | #keyPath(...) | test.swift:866:9:866:9 | myKeyPath |
+| test.swift:866:21:866:29 | enter #keyPath(...) | test.swift:866:27:866:29 | KeyPathComponent |
+| test.swift:866:27:866:29 | [post] KeyPathComponent | test.swift:866:21:866:29 | [post] enter #keyPath(...) |
+| test.swift:871:24:871:31 | [...] | test.swift:871:24:871:31 | [...] |
+| test.swift:872:28:872:31 | [...] | test.swift:872:28:872:31 | [...] |
+| test.swift:873:21:873:31 | [...] | test.swift:873:21:873:31 | [...] |

swift/ql/test/library-tests/dataflow/dataflow/test.swift

@@ -839,4 +839,36 @@ func testNestedKeyPathWrite() {
   var f = \S2.s.x
   s2[keyPath: f] = source()
   sink(arg: s2.s.x) // $ flow=840
-}
+}
+
+func testVarargs1(args: Int...) {
+    sink(arg: args)
+    sink(arg: args[0]) // $ flow=871
+}
+
+func testVarargs2(_ v: Int, _ args: Int...) {
+    sink(arg: v) // $ flow=872
+    sink(arg: args)
+    sink(arg: args[0])
+    sink(arg: args[1])
+}
+
+func testVarargs3(_ v: Int, _ args: Int...) {
+    sink(arg: v)
+    sink(arg: args)
+    sink(arg: args[0]) // $ SPURIOUS: flow=873
+    sink(arg: args[1]) // $ flow=873
+
+    for arg in args {
+        sink(arg: arg) // $ MISSING: flow=873
+    }
+
+    let myKeyPath = \[Int][1]
+    sink(arg: args[keyPath: myKeyPath]) // $ flow=873
+}
+
+func testVarargsCaller() {
+    testVarargs1(args: source())
+    testVarargs2(source(), 2, 3)
+    testVarargs3(1, 2, source())
+}

swift/ql/test/library-tests/dataflow/taint/libraries/string.swift

@@ -226,8 +226,8 @@ func taintThroughSimpleStringOperations() {
   sink(arg: String(format: tainted, locale: nil, 1, 2, 3)) // $ tainted=217
   sink(arg: String(format: tainted, locale: nil, arguments: [])) // $ tainted=217
   sink(arg: String.localizedStringWithFormat(tainted, 1, 2, 3)) // $ tainted=217
-  sink(arg: String(format: "%s", tainted)) // $ MISSING: tainted=217
-  sink(arg: String(format: "%i %i %i", 1, 2, taintedInt)) // $ MISSING: tainted=218
+  sink(arg: String(format: "%s", tainted)) // $ tainted=217
+  sink(arg: String(format: "%i %i %i", 1, 2, taintedInt)) // $ tainted=218
 
   sink(arg: String(repeating: clean, count: 2))
   sink(arg: String(repeating: tainted, count: 2)) // $ tainted=217

swift/ql/test/query-tests/Security/CWE-312/cleartextLoggingTest.swift

@@ -84,20 +84,20 @@ struct Logger {
 // --- tests ---
 
 func test1(password: String, passwordHash : String, passphrase: String, pass_phrase: String) {
-	print(password) // $ MISSING: hasCleartextLogging=87
-	print(password, separator: "") // $ MISSING: $ hasCleartextLogging=88
+	print(password) // $ hasCleartextLogging=87
+	print(password, separator: "") // $ $ hasCleartextLogging=88
 	print("", separator: password) // $ hasCleartextLogging=89
-	print(password, separator: "", terminator: "") // $ MISSING: hasCleartextLogging=90
+	print(password, separator: "", terminator: "") // $ hasCleartextLogging=90
 	print("", separator: password, terminator: "") // $ hasCleartextLogging=91
 	print("", separator: "", terminator: password) // $ hasCleartextLogging=92
     print(passwordHash) // Safe
 
     NSLog(password) // $ hasCleartextLogging=95
-    NSLog("%@", password as! CVarArg) // $ MISSING: hasCleartextLogging=96
-    NSLog("%@ %@", "" as! CVarArg, password as! CVarArg) // $ MISSING: hasCleartextLogging=97
+    NSLog("%@", password as! CVarArg) // $ hasCleartextLogging=96
+    NSLog("%@ %@", "" as! CVarArg, password as! CVarArg) // $ hasCleartextLogging=97
     NSLog("\(password)") // $ hasCleartextLogging=98
-    NSLogv("%@", getVaList([password as! CVarArg])) // $ MISSING: hasCleartextLogging=99
-    NSLogv("%@ %@", getVaList(["" as! CVarArg, password as! CVarArg])) // $ MISSING: hasCleartextLogging=100
+    NSLogv("%@", getVaList([password as! CVarArg])) // $ hasCleartextLogging=99
+    NSLogv("%@ %@", getVaList(["" as! CVarArg, password as! CVarArg])) // $ hasCleartextLogging=100
     NSLog(passwordHash) // SAfe
     NSLogv("%@", getVaList([passwordHash as! CVarArg])) // Safe