Swift: Fix false positives in the swift/cleartext-transmission query

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

@@ -7,6 +7,7 @@ import swift
 import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
 import codeql.swift.dataflow.ExternalFlow
+import codeql.swift.dataflow.TaintTracking
 
 /**
  * A dataflow sink for cleartext transmission vulnerabilities. That is,
@@ -48,6 +49,48 @@ private class AlamofireTransmittedSink extends CleartextTransmissionSink {
   }
 }
 
+/**
+ * A call to `URL.init`.
+ */
+private predicate urlInit(CallExpr urlInit, Expr withString) {
+  urlInit
+      .getStaticTarget()
+      .(Method)
+      .hasQualifiedName("URL", ["init(string:)", "init(string:relativeTo:)"]) and
+  urlInit.getArgument(0).getExpr() = withString
+}
+
+/**
+ * A data flow configuration for tracking string literals representing `tel:` and similar
+ * URLs to creation of URL objects.
+ */
+private module ExcludeUrlConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) {
+    node.asExpr()
+        .(StringLiteralExpr)
+        .getValue()
+        .regexpMatch("^(mailto|file|tel|telprompt|callto|sms):.*")
+  }
+
+  predicate isSink(DataFlow::Node node) { urlInit(_, node.asExpr()) }
+}
+
+private module ExcludeUrlFlow = TaintTracking::Global<ExcludeUrlConfig>;
+
+/**
+ * A `URL` that is a sink for this query. Not all URLs are considered sinks, depending
+ * on their content.
+ */
+private class UrlTransmittedSink extends CleartextTransmissionSink {
+  UrlTransmittedSink() {
+    urlInit(_, this.asExpr()) and
+    // exclude `tel:` and similar URLs. These URLs necessarily contain
+    // sensitive data which you expect to transmit only by making the
+    // phone call (or similar operation).
+    not ExcludeUrlFlow::flow(_, this)
+  }
+}
+
 /**
  * A barrier for cleartext transmission vulnerabilities.
  *  - encryption; encrypted values are not cleartext.
@@ -81,12 +124,6 @@ private class DefaultCleartextTransmissionSink extends CleartextTransmissionSink
 private class TransmissionSinks extends SinkModelCsv {
   override predicate row(string row) {
     row =
-      [
-        ";NWConnection;true;send(content:contentContext:isComplete:completion:);;;Argument[0];transmission",
-        // an `Expr` that is used to form a `URL` is very likely to be transmitted over a network, because
-        // that's what URLs are for.
-        ";URL;true;init(string:);;;Argument[0];transmission",
-        ";URL;true;init(string:relativeTo:);;;Argument[0];transmission",
-      ]
+      ";NWConnection;true;send(content:contentContext:isComplete:completion:);;;Argument[0];transmission"
   }
 }

swift/ql/src/change-notes/2024-08-12-cleartext-transmission.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* False positive results from the `swift/cleartext-transmission` ("Cleartext transmission of sensitive information") query involving `tel:`, `mailto:` and similar URLs have been fixed.

swift/ql/test/query-tests/Security/CWE-311/CleartextTransmission.expected

@@ -32,6 +32,9 @@ edges
 | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) | testURL.swift:106:20:106:20 | "..." | provenance |  |
 | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | testURL.swift:105:6:105:10 | let ...? [some:0] | provenance |  |
 | testURL.swift:105:32:105:32 | data | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | provenance |  |
+| testURL.swift:116:52:116:52 | email | testURL.swift:116:18:116:18 | "..." | provenance |  |
+| testURL.swift:123:52:123:52 | phone_number | testURL.swift:123:18:123:18 | "..." | provenance |  |
+| testURL.swift:132:39:132:39 | account_no | testURL.swift:132:18:132:18 | "..." | provenance |  |
 nodes
 | file://:0:0:0:0 | .value | semmle.label | .value |
 | file://:0:0:0:0 | self | semmle.label | self |
@@ -90,6 +93,12 @@ nodes
 | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | semmle.label | call to String.init(data:encoding:) [some:0] |
 | testURL.swift:105:32:105:32 | data | semmle.label | data |
 | testURL.swift:106:20:106:20 | "..." | semmle.label | "..." |
+| testURL.swift:116:18:116:18 | "..." | semmle.label | "..." |
+| testURL.swift:116:52:116:52 | email | semmle.label | email |
+| testURL.swift:123:18:123:18 | "..." | semmle.label | "..." |
+| testURL.swift:123:52:123:52 | phone_number | semmle.label | phone_number |
+| testURL.swift:132:18:132:18 | "..." | semmle.label | "..." |
+| testURL.swift:132:39:132:39 | account_no | semmle.label | account_no |
 subpaths
 | testSend.swift:60:17:60:17 | password | testSend.swift:41:10:41:18 | data | testSend.swift:41:45:41:45 | data | testSend.swift:60:13:60:25 | call to pad(_:) |
 | testSend.swift:94:27:94:30 | .password | testSend.swift:86:7:86:7 | self | file://:0:0:0:0 | .value | testSend.swift:94:27:94:39 | .value |
@@ -121,3 +130,6 @@ subpaths
 | testURL.swift:75:18:75:69 | ... .+(_:_:) ... | testURL.swift:75:53:75:69 | call to get_cert_string() | testURL.swift:75:18:75:69 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:75:53:75:69 | call to get_cert_string() | call to get_cert_string() |
 | testURL.swift:96:18:96:18 | "..." | testURL.swift:96:51:96:51 | certificate | testURL.swift:96:18:96:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:96:51:96:51 | certificate | certificate |
 | testURL.swift:106:20:106:20 | "..." | testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | testURL.swift:106:20:106:20 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | call to SecKeyCopyExternalRepresentation(_:_:) |
+| testURL.swift:116:18:116:18 | "..." | testURL.swift:116:52:116:52 | email | testURL.swift:116:18:116:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:116:52:116:52 | email | email |
+| testURL.swift:123:18:123:18 | "..." | testURL.swift:123:52:123:52 | phone_number | testURL.swift:123:18:123:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:123:52:123:52 | phone_number | phone_number |
+| testURL.swift:132:18:132:18 | "..." | testURL.swift:132:39:132:39 | account_no | testURL.swift:132:18:132:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:132:39:132:39 | account_no | account_no |

swift/ql/test/query-tests/Security/CWE-311/SensitiveExprs.expected

@@ -182,3 +182,14 @@
 | testURL.swift:75:53:75:69 | call to get_cert_string() | label:get_cert_string, type:credential |
 | testURL.swift:96:51:96:51 | certificate | label:certificate, type:credential |
 | testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | label:password, type:password |
+| testURL.swift:116:52:116:52 | email | label:email, type:private information |
+| testURL.swift:117:28:117:28 | email | label:email, type:private information |
+| testURL.swift:118:53:118:53 | secret_key | label:secret_key, type:credential |
+| testURL.swift:119:60:119:60 | email | label:email, type:private information |
+| testURL.swift:123:52:123:52 | phone_number | label:phone_number, type:private information |
+| testURL.swift:124:25:124:25 | phone_number | label:phone_number, type:private information |
+| testURL.swift:125:31:125:31 | phone_number | label:phone_number, type:private information |
+| testURL.swift:126:28:126:28 | phone_number | label:phone_number, type:private information |
+| testURL.swift:127:25:127:25 | phone_number | label:phone_number, type:private information |
+| testURL.swift:131:37:131:37 | account_no | label:account_no, type:private information |
+| testURL.swift:132:39:132:39 | account_no | label:account_no, type:private information |

swift/ql/test/query-tests/Security/CWE-311/testURL.swift

@@ -107,3 +107,27 @@ func test4(key: SecKey) {
 		}
 	}
 }
+
+func test5() {
+	// variant URL types...
+	let email = get_string()
+	let secret_key = get_string()
+
+	_ = URL(string: "http://example.com/login?email=\(email)"); // BAD
+	_ = URL(string: "mailto:\(email)"); // GOOD (revealing your e-amil address in an e-mail is expected)
+	_ = URL(string: "mailto:[email protected]?subject=\(secret_key)"); // BAD [NOT DETECTED]
+	_ = URL(string: "mailto:[email protected]?subject=foo&cc=\(email)"); // GOOD
+
+	let phone_number = get_string()
+
+	_ = URL(string: "http://example.com/profile?tel=\(phone_number)"); // BAD
+	_ = URL(string: "tel:\(phone_number)") // GOOD
+	_ = URL(string: "telprompt:\(phone_number)") // GOOD
+	_ = URL(string: "callto:\(phone_number)") // GOOD
+	_ = URL(string: "sms:\(phone_number)") // GOOD
+
+	let account_no = get_string()
+
+	_ = URL(string: "file:///foo/bar/\(account_no).csv") // GOOD (local, so not transmitted)
+	_ = URL(string: "ftp://example.com/\(account_no).csv") // BAD
+}