Skip to content

Commit 446c992

Browse files
geoffw0geoffw0
committed
Swift: Exclude tel:, mailto: and similar URLs from the query.
1 parent 897bfb5 commit 446c992

File tree

3 files changed

+44
-50
lines changed

3 files changed

+44
-50
lines changed

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

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import swift
77
import codeql.swift.security.SensitiveExprs
88
import codeql.swift.dataflow.DataFlow
99
import codeql.swift.dataflow.ExternalFlow
10+
import codeql.swift.dataflow.TaintTracking
1011

1112
/**
1213
* A dataflow sink for cleartext transmission vulnerabilities. That is,
@@ -48,20 +49,45 @@ private class AlamofireTransmittedSink extends CleartextTransmissionSink {
4849
}
4950
}
5051

52+
/**
53+
* A call to `URL.init`.
54+
*/
55+
private predicate urlInit(CallExpr urlInit, Expr withString) {
56+
urlInit
57+
.getStaticTarget()
58+
.(Method)
59+
.hasQualifiedName("URL", ["init(string:)", "init(string:relativeTo:)"]) and
60+
urlInit.getArgument(0).getExpr() = withString
61+
}
62+
63+
/**
64+
* A data flow configuration for tracking string literals representing `tel:` and similar
65+
* URLs to creation of URL objects.
66+
*/
67+
private module ExcludeUrlConfig implements DataFlow::ConfigSig {
68+
predicate isSource(DataFlow::Node node) {
69+
node.asExpr()
70+
.(StringLiteralExpr)
71+
.getValue()
72+
.regexpMatch("^(mailto|file|tel|telprompt|callto|sms):.*")
73+
}
74+
75+
predicate isSink(DataFlow::Node node) { urlInit(_, node.asExpr()) }
76+
}
77+
78+
private module ExcludeUrlFlow = TaintTracking::Global<ExcludeUrlConfig>;
79+
5180
/**
5281
* A `URL` that is a sink for this query. Not all URLs are considered sinks, depending
5382
* on their content.
5483
*/
55-
private class URLTransmittedSink extends CleartextTransmissionSink {
56-
URLTransmittedSink() {
57-
// sinks are the first argument containing the URL, and the `parameters`
58-
// and `headers` arguments to appropriate methods of `Session`.
59-
exists(CallExpr call |
60-
call.getStaticTarget()
61-
.(Method)
62-
.hasQualifiedName("URL", ["init(string:)", "init(string:relativeTo:)"]) and
63-
call.getArgument(0).getExpr() = this.asExpr()
64-
)
84+
private class UrlTransmittedSink extends CleartextTransmissionSink {
85+
UrlTransmittedSink() {
86+
urlInit(_, this.asExpr()) and
87+
// exclude `tel:` and similar URLs. These URLs necessarily contain
88+
// sensitive data which you expect to transmit only by making the
89+
// phone call (or similar operation).
90+
not ExcludeUrlFlow::flow(_, this)
6591
}
6692
}
6793

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

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,7 @@ edges
3333
| testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | testURL.swift:105:6:105:10 | let ...? [some:0] | provenance | |
3434
| testURL.swift:105:32:105:32 | data | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | provenance | |
3535
| testURL.swift:116:52:116:52 | email | testURL.swift:116:18:116:18 | "..." | provenance | |
36-
| testURL.swift:117:28:117:28 | email | testURL.swift:117:18:117:18 | "..." | provenance | |
37-
| testURL.swift:118:53:118:53 | secret_key | testURL.swift:118:18:118:18 | "..." | provenance | |
38-
| testURL.swift:119:60:119:60 | email | testURL.swift:119:18:119:18 | "..." | provenance | |
3936
| testURL.swift:123:52:123:52 | phone_number | testURL.swift:123:18:123:18 | "..." | provenance | |
40-
| testURL.swift:124:25:124:25 | phone_number | testURL.swift:124:18:124:18 | "..." | provenance | |
41-
| testURL.swift:125:31:125:31 | phone_number | testURL.swift:125:18:125:18 | "..." | provenance | |
42-
| testURL.swift:126:28:126:28 | phone_number | testURL.swift:126:18:126:18 | "..." | provenance | |
43-
| testURL.swift:127:25:127:25 | phone_number | testURL.swift:127:18:127:18 | "..." | provenance | |
44-
| testURL.swift:131:37:131:37 | account_no | testURL.swift:131:18:131:18 | "..." | provenance | |
4537
| testURL.swift:132:39:132:39 | account_no | testURL.swift:132:18:132:18 | "..." | provenance | |
4638
nodes
4739
| file://:0:0:0:0 | .value | semmle.label | .value |
@@ -103,24 +95,8 @@ nodes
10395
| testURL.swift:106:20:106:20 | "..." | semmle.label | "..." |
10496
| testURL.swift:116:18:116:18 | "..." | semmle.label | "..." |
10597
| testURL.swift:116:52:116:52 | email | semmle.label | email |
106-
| testURL.swift:117:18:117:18 | "..." | semmle.label | "..." |
107-
| testURL.swift:117:28:117:28 | email | semmle.label | email |
108-
| testURL.swift:118:18:118:18 | "..." | semmle.label | "..." |
109-
| testURL.swift:118:53:118:53 | secret_key | semmle.label | secret_key |
110-
| testURL.swift:119:18:119:18 | "..." | semmle.label | "..." |
111-
| testURL.swift:119:60:119:60 | email | semmle.label | email |
11298
| testURL.swift:123:18:123:18 | "..." | semmle.label | "..." |
11399
| testURL.swift:123:52:123:52 | phone_number | semmle.label | phone_number |
114-
| testURL.swift:124:18:124:18 | "..." | semmle.label | "..." |
115-
| testURL.swift:124:25:124:25 | phone_number | semmle.label | phone_number |
116-
| testURL.swift:125:18:125:18 | "..." | semmle.label | "..." |
117-
| testURL.swift:125:31:125:31 | phone_number | semmle.label | phone_number |
118-
| testURL.swift:126:18:126:18 | "..." | semmle.label | "..." |
119-
| testURL.swift:126:28:126:28 | phone_number | semmle.label | phone_number |
120-
| testURL.swift:127:18:127:18 | "..." | semmle.label | "..." |
121-
| testURL.swift:127:25:127:25 | phone_number | semmle.label | phone_number |
122-
| testURL.swift:131:18:131:18 | "..." | semmle.label | "..." |
123-
| testURL.swift:131:37:131:37 | account_no | semmle.label | account_no |
124100
| testURL.swift:132:18:132:18 | "..." | semmle.label | "..." |
125101
| testURL.swift:132:39:132:39 | account_no | semmle.label | account_no |
126102
subpaths
@@ -155,13 +131,5 @@ subpaths
155131
| 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 |
156132
| 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(_:_:) |
157133
| 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 |
158-
| testURL.swift:117:18:117:18 | "..." | testURL.swift:117:28:117:28 | email | testURL.swift:117:18:117:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:117:28:117:28 | email | email |
159-
| testURL.swift:118:18:118:18 | "..." | testURL.swift:118:53:118:53 | secret_key | testURL.swift:118:18:118:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:118:53:118:53 | secret_key | secret_key |
160-
| testURL.swift:119:18:119:18 | "..." | testURL.swift:119:60:119:60 | email | testURL.swift:119:18:119:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:119:60:119:60 | email | email |
161134
| 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 |
162-
| testURL.swift:124:18:124:18 | "..." | testURL.swift:124:25:124:25 | phone_number | testURL.swift:124:18:124:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:124:25:124:25 | phone_number | phone_number |
163-
| testURL.swift:125:18:125:18 | "..." | testURL.swift:125:31:125:31 | phone_number | testURL.swift:125:18:125:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:125:31:125:31 | phone_number | phone_number |
164-
| testURL.swift:126:18:126:18 | "..." | testURL.swift:126:28:126:28 | phone_number | testURL.swift:126:18:126:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:126:28:126:28 | phone_number | phone_number |
165-
| testURL.swift:127:18:127:18 | "..." | testURL.swift:127:25:127:25 | phone_number | testURL.swift:127:18:127:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:127:25:127:25 | phone_number | phone_number |
166-
| testURL.swift:131:18:131:18 | "..." | testURL.swift:131:37:131:37 | account_no | testURL.swift:131:18:131:18 | "..." | This operation transmits '"..."', which may contain unencrypted sensitive data from $@. | testURL.swift:131:37:131:37 | account_no | account_no |
167135
| 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/testURL.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,20 +114,20 @@ func test5() {
114114
let secret_key = get_string()
115115

116116
_ = URL(string: "http://example.com/login?email=\(email)"); // BAD
117-
_ = URL(string: "mailto:\(email)"); // GOOD (revealing your e-amil address in an e-mail is expected) [FALSE POSITIVE]
118-
_ = URL(string: "mailto:[email protected]?subject=\(secret_key)"); // BAD
119-
_ = URL(string: "mailto:[email protected]?subject=foo&cc=\(email)"); // GOOD [FALSE POSITIVE]
117+
_ = URL(string: "mailto:\(email)"); // GOOD (revealing your e-amil address in an e-mail is expected)
118+
_ = URL(string: "mailto:[email protected]?subject=\(secret_key)"); // BAD [NOT DETECTED]
119+
_ = URL(string: "mailto:[email protected]?subject=foo&cc=\(email)"); // GOOD
120120

121121
let phone_number = get_string()
122122

123123
_ = URL(string: "http://example.com/profile?tel=\(phone_number)"); // BAD
124-
_ = URL(string: "tel:\(phone_number)") // GOOD [FALSE POSITIVE]
125-
_ = URL(string: "telprompt:\(phone_number)") // GOOD [FALSE POSITIVE]
126-
_ = URL(string: "callto:\(phone_number)") // GOOD [FALSE POSITIVE]
127-
_ = URL(string: "sms:\(phone_number)") // GOOD [FALSE POSITIVE]
124+
_ = URL(string: "tel:\(phone_number)") // GOOD
125+
_ = URL(string: "telprompt:\(phone_number)") // GOOD
126+
_ = URL(string: "callto:\(phone_number)") // GOOD
127+
_ = URL(string: "sms:\(phone_number)") // GOOD
128128

129129
let account_no = get_string()
130130

131-
_ = URL(string: "file:///foo/bar/\(account_no).csv") // GOOD (local, so not transmitted) [FALSE POSITIVE]
131+
_ = URL(string: "file:///foo/bar/\(account_no).csv") // GOOD (local, so not transmitted)
132132
_ = URL(string: "ftp://example.com/\(account_no).csv") // BAD
133133
}

0 commit comments

Comments
 (0)