Skip to content

Commit 5095031

Browse files
geoffw0geoffw0
committed
Swift: Model SecKeyCopyExternalRepresentation as an explicit sensitive data source.
1 parent 1d903c5 commit 5095031

File tree

6 files changed

+42
-1
lines changed

6 files changed

+42
-1
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* Provides models for standard library Swift classses related to security
3+
* (certificate, key and trust services).
4+
*/
5+
6+
import swift
7+
private import codeql.swift.dataflow.ExternalFlow
8+
9+
private class SensitiveSources extends SourceModelCsv {
10+
override predicate row(string row) {
11+
row = ";;false;SecKeyCopyExternalRepresentation(_:_:);;;ReturnValue;sensitive-credential"
12+
}
13+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ private import NsUrl
1919
private import Numeric
2020
private import RawRepresentable
2121
private import PointerTypes
22+
private import Security
2223
private import Sequence
2324
private import Set
2425
private import Stream

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
import swift
88
import internal.SensitiveDataHeuristics
9+
private import codeql.swift.dataflow.DataFlow
10+
private import codeql.swift.dataflow.ExternalFlow
911

1012
private newtype TSensitiveDataType =
1113
TCredential() or
@@ -172,6 +174,18 @@ class SensitiveExpr extends Expr {
172174
) and
173175
// do not mark as sensitive it if it is probably safe
174176
not label.regexpMatch(regexpProbablySafe())
177+
or
178+
(
179+
// modelled sensitive credential
180+
sourceNode(DataFlow::exprNode(this), "sensitive-credential") and
181+
sensitiveType = TCredential() and
182+
label = "credential"
183+
or
184+
// modelled sensitive private information
185+
sourceNode(DataFlow::exprNode(this), "sensitive-private-info") and
186+
sensitiveType = TPrivateInfo() and
187+
label = "private information"
188+
)
175189
}
176190

177191
/**

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ edges
2323
| testURL.swift:73:52:73:67 | call to get_secret_key() | testURL.swift:73:18:73:67 | ... .+(_:_:) ... |
2424
| testURL.swift:75:53:75:69 | call to get_cert_string() | testURL.swift:75:18:75:69 | ... .+(_:_:) ... |
2525
| testURL.swift:96:51:96:51 | certificate | testURL.swift:96:18:96:18 | "..." |
26+
| testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | testURL.swift:105:32:105:32 | data |
27+
| testURL.swift:105:6:105:10 | let ...? [some:0] | testURL.swift:105:10:105:10 | string |
28+
| testURL.swift:105:10:105:10 | string | testURL.swift:106:20:106:20 | "..." |
29+
| testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | testURL.swift:105:6:105:10 | let ...? [some:0] |
30+
| testURL.swift:105:32:105:32 | data | testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] |
2631
nodes
2732
| file://:0:0:0:0 | .value | semmle.label | .value |
2833
| file://:0:0:0:0 | self | semmle.label | self |
@@ -74,6 +79,12 @@ nodes
7479
| testURL.swift:75:53:75:69 | call to get_cert_string() | semmle.label | call to get_cert_string() |
7580
| testURL.swift:96:18:96:18 | "..." | semmle.label | "..." |
7681
| testURL.swift:96:51:96:51 | certificate | semmle.label | certificate |
82+
| testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | semmle.label | call to SecKeyCopyExternalRepresentation(_:_:) |
83+
| testURL.swift:105:6:105:10 | let ...? [some:0] | semmle.label | let ...? [some:0] |
84+
| testURL.swift:105:10:105:10 | string | semmle.label | string |
85+
| testURL.swift:105:19:105:53 | call to String.init(data:encoding:) [some:0] | semmle.label | call to String.init(data:encoding:) [some:0] |
86+
| testURL.swift:105:32:105:32 | data | semmle.label | data |
87+
| testURL.swift:106:20:106:20 | "..." | semmle.label | "..." |
7788
subpaths
7889
| 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(_:) |
7990
| 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 |
@@ -104,3 +115,4 @@ subpaths
104115
| testURL.swift:73:18:73:67 | ... .+(_:_:) ... | testURL.swift:73:52:73:67 | call to get_secret_key() | testURL.swift:73:18:73:67 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:73:52:73:67 | call to get_secret_key() | call to get_secret_key() |
105116
| 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() |
106117
| 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 |
118+
| 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(_:_:) |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,4 @@
175175
| testURL.swift:73:52:73:67 | call to get_secret_key() | label:get_secret_key, type:credential |
176176
| testURL.swift:75:53:75:69 | call to get_cert_string() | label:get_cert_string, type:credential |
177177
| testURL.swift:96:51:96:51 | certificate | label:certificate, type:credential |
178+
| testURL.swift:104:16:104:57 | call to SecKeyCopyExternalRepresentation(_:_:) | label:credential, type:credential |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func test3() {
103103
func test4(key: SecKey) {
104104
if let data = SecKeyCopyExternalRepresentation(key, nil) as? Data {
105105
if let string = String(data: data, encoding: .utf8) {
106-
_ = URL(string: "http://example.com/login?tok=\(string)"); // BAD [NOT DETECTED]
106+
_ = URL(string: "http://example.com/login?tok=\(string)"); // BAD
107107
}
108108
}
109109
}

0 commit comments

Comments
 (0)