Skip to content

Commit ae0fcf7

Browse files
geoffw0geoffw0
committed
Swift: Expand the additional taint step from the cleartext storage database query to the other sensitive data queries.
1 parent aa5820c commit ae0fcf7

File tree

8 files changed

+58
-4
lines changed

8 files changed

+58
-4
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@ private class OsLogPrivacyRef extends MemberRefExpr {
8383
predicate isPublic() { optionName = "public" }
8484
}
8585

86+
/**
87+
* An additional taint step for cleartext logging vulnerabilities.
88+
*/
89+
private class CleartextLoggingFieldAdditionalFlowStep extends CleartextLoggingAdditionalFlowStep {
90+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
91+
// if an object is sensitive, its fields are always sensitive.
92+
nodeTo.asExpr().(MemberRefExpr).getBase() = nodeFrom.asExpr()
93+
}
94+
}
95+
8696
private class LoggingSinks extends SinkModelCsv {
8797
override predicate row(string row) {
8898
row =

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ private class CleartextStoragePreferencesDefaultBarrier extends CleartextStorage
8989
}
9090
}
9191

92+
/**
93+
* An additional taint step for cleartext preferences storage vulnerabilities.
94+
*/
95+
private class CleartextStoragePreferencesFieldAdditionalFlowStep extends CleartextStoragePreferencesAdditionalFlowStep
96+
{
97+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
98+
// if an object is sensitive, its fields are always sensitive.
99+
nodeTo.asExpr().(MemberRefExpr).getBase() = nodeFrom.asExpr()
100+
}
101+
}
102+
92103
/**
93104
* A sink defined in a CSV model.
94105
*/

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,17 @@ private class CleartextTransmissionDefaultBarrier extends CleartextTransmissionB
6060
}
6161
}
6262

63+
/**
64+
* An additional taint step for cleartext transmission vulnerabilities.
65+
*/
66+
private class CleartextTransmissionFieldAdditionalFlowStep extends CleartextTransmissionAdditionalFlowStep
67+
{
68+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
69+
// if an object is sensitive, its fields are always sensitive.
70+
nodeTo.asExpr().(MemberRefExpr).getBase() = nodeFrom.asExpr()
71+
}
72+
}
73+
6374
/**
6475
* A sink defined in a CSV model.
6576
*/

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| file://:0:0:0:0 | self | file://:0:0:0:0 | .value |
23
| testAlamofire.swift:150:45:150:45 | password | testAlamofire.swift:150:13:150:45 | ... .+(_:_:) ... |
34
| testAlamofire.swift:152:51:152:51 | password | testAlamofire.swift:152:19:152:51 | ... .+(_:_:) ... |
45
| testAlamofire.swift:154:38:154:38 | email | testAlamofire.swift:154:14:154:46 | ... .+(_:_:) ... |
@@ -10,13 +11,18 @@ edges
1011
| testSend.swift:60:13:60:25 | call to pad(_:) | testSend.swift:67:27:67:27 | str3 |
1112
| testSend.swift:60:17:60:17 | password | testSend.swift:41:10:41:18 | data |
1213
| testSend.swift:60:17:60:17 | password | testSend.swift:60:13:60:25 | call to pad(_:) |
14+
| testSend.swift:86:7:86:7 | self | file://:0:0:0:0 | self |
15+
| testSend.swift:94:27:94:30 | .password | testSend.swift:86:7:86:7 | self |
16+
| testSend.swift:94:27:94:30 | .password | testSend.swift:94:27:94:39 | .value |
1317
| testURL.swift:17:54:17:54 | passwd | testURL.swift:17:22:17:54 | ... .+(_:_:) ... |
1418
| testURL.swift:19:55:19:55 | account_no | testURL.swift:19:22:19:55 | ... .+(_:_:) ... |
1519
| testURL.swift:20:55:20:55 | credit_card_no | testURL.swift:20:22:20:55 | ... .+(_:_:) ... |
1620
| testURL.swift:28:55:28:55 | e_mail | testURL.swift:28:22:28:55 | ... .+(_:_:) ... |
1721
| testURL.swift:30:57:30:57 | a_homeaddr_z | testURL.swift:30:22:30:57 | ... .+(_:_:) ... |
1822
| testURL.swift:32:55:32:55 | resident_ID | testURL.swift:32:22:32:55 | ... .+(_:_:) ... |
1923
nodes
24+
| file://:0:0:0:0 | .value | semmle.label | .value |
25+
| file://:0:0:0:0 | self | semmle.label | self |
2026
| testAlamofire.swift:150:13:150:45 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
2127
| testAlamofire.swift:150:45:150:45 | password | semmle.label | password |
2228
| testAlamofire.swift:152:19:152:51 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
@@ -43,6 +49,9 @@ nodes
4349
| testSend.swift:78:27:78:30 | .CarePlanID | semmle.label | .CarePlanID |
4450
| testSend.swift:79:27:79:30 | .BankCardNo | semmle.label | .BankCardNo |
4551
| testSend.swift:80:27:80:30 | .MyCreditRating | semmle.label | .MyCreditRating |
52+
| testSend.swift:86:7:86:7 | self | semmle.label | self |
53+
| testSend.swift:94:27:94:30 | .password | semmle.label | .password |
54+
| testSend.swift:94:27:94:39 | .value | semmle.label | .value |
4655
| testURL.swift:17:22:17:54 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
4756
| testURL.swift:17:54:17:54 | passwd | semmle.label | passwd |
4857
| testURL.swift:19:22:19:55 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
@@ -58,6 +67,7 @@ nodes
5867
| testURL.swift:32:55:32:55 | resident_ID | semmle.label | resident_ID |
5968
subpaths
6069
| 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(_:) |
70+
| 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 |
6171
#select
6272
| testAlamofire.swift:150:13:150:45 | ... .+(_:_:) ... | testAlamofire.swift:150:45:150:45 | password | testAlamofire.swift:150:13:150:45 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testAlamofire.swift:150:45:150:45 | password | password |
6373
| testAlamofire.swift:152:19:152:51 | ... .+(_:_:) ... | testAlamofire.swift:152:51:152:51 | password | testAlamofire.swift:152:19:152:51 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testAlamofire.swift:152:51:152:51 | password | password |
@@ -74,6 +84,7 @@ subpaths
7484
| testSend.swift:78:27:78:30 | .CarePlanID | testSend.swift:78:27:78:30 | .CarePlanID | testSend.swift:78:27:78:30 | .CarePlanID | This operation transmits '.CarePlanID', which may contain unencrypted sensitive data from $@. | testSend.swift:78:27:78:30 | .CarePlanID | .CarePlanID |
7585
| testSend.swift:79:27:79:30 | .BankCardNo | testSend.swift:79:27:79:30 | .BankCardNo | testSend.swift:79:27:79:30 | .BankCardNo | This operation transmits '.BankCardNo', which may contain unencrypted sensitive data from $@. | testSend.swift:79:27:79:30 | .BankCardNo | .BankCardNo |
7686
| testSend.swift:80:27:80:30 | .MyCreditRating | testSend.swift:80:27:80:30 | .MyCreditRating | testSend.swift:80:27:80:30 | .MyCreditRating | This operation transmits '.MyCreditRating', which may contain unencrypted sensitive data from $@. | testSend.swift:80:27:80:30 | .MyCreditRating | .MyCreditRating |
87+
| testSend.swift:94:27:94:39 | .value | testSend.swift:94:27:94:30 | .password | testSend.swift:94:27:94:39 | .value | This operation transmits '.value', which may contain unencrypted sensitive data from $@. | testSend.swift:94:27:94:30 | .password | .password |
7788
| testURL.swift:17:22:17:54 | ... .+(_:_:) ... | testURL.swift:17:54:17:54 | passwd | testURL.swift:17:22:17:54 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:17:54:17:54 | passwd | passwd |
7889
| testURL.swift:19:22:19:55 | ... .+(_:_:) ... | testURL.swift:19:55:19:55 | account_no | testURL.swift:19:22:19:55 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:19:55:19:55 | account_no | account_no |
7990
| testURL.swift:20:22:20:55 | ... .+(_:_:) ... | testURL.swift:20:55:20:55 | credit_card_no | testURL.swift:20:22:20:55 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:20:55:20:55 | credit_card_no | credit_card_no |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,6 @@ struct MyOuter {
9191
}
9292

9393
func test3(mo : MyOuter, connection : NWConnection) {
94-
connection.send(content: mo.password.value, completion: .idempotent) // BAD [NOT DETECTED]
94+
connection.send(content: mo.password.value, completion: .idempotent) // BAD
9595
connection.send(content: mo.harmless.value, completion: .idempotent) // GOOD
9696
}

‎swift/ql/test/query-tests/Security/CWE-312/CleartextStoragePreferences.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| file://:0:0:0:0 | self | file://:0:0:0:0 | .value |
23
| testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x | testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x |
34
| testNSUbiquitousKeyValueStore.swift:44:10:44:22 | call to getPassword() | testNSUbiquitousKeyValueStore.swift:45:40:45:40 | y |
45
| testNSUbiquitousKeyValueStore.swift:55:10:55:10 | passwd | testNSUbiquitousKeyValueStore.swift:59:40:59:40 | x |
@@ -9,7 +10,12 @@ edges
910
| testUserDefaults.swift:55:10:55:10 | passwd | testUserDefaults.swift:59:28:59:28 | x |
1011
| testUserDefaults.swift:56:10:56:10 | passwd | testUserDefaults.swift:60:28:60:28 | y |
1112
| testUserDefaults.swift:57:10:57:10 | passwd | testUserDefaults.swift:61:28:61:28 | z |
13+
| testUserDefaults.swift:74:7:74:7 | self | file://:0:0:0:0 | self |
14+
| testUserDefaults.swift:82:28:82:31 | .password | testUserDefaults.swift:74:7:74:7 | self |
15+
| testUserDefaults.swift:82:28:82:31 | .password | testUserDefaults.swift:82:28:82:40 | .value |
1216
nodes
17+
| file://:0:0:0:0 | .value | semmle.label | .value |
18+
| file://:0:0:0:0 | self | semmle.label | self |
1319
| testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | semmle.label | password |
1420
| testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x | semmle.label | x |
1521
| testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | semmle.label | x |
@@ -34,7 +40,11 @@ nodes
3440
| testUserDefaults.swift:59:28:59:28 | x | semmle.label | x |
3541
| testUserDefaults.swift:60:28:60:28 | y | semmle.label | y |
3642
| testUserDefaults.swift:61:28:61:28 | z | semmle.label | z |
43+
| testUserDefaults.swift:74:7:74:7 | self | semmle.label | self |
44+
| testUserDefaults.swift:82:28:82:31 | .password | semmle.label | .password |
45+
| testUserDefaults.swift:82:28:82:40 | .value | semmle.label | .value |
3746
subpaths
47+
| testUserDefaults.swift:82:28:82:31 | .password | testUserDefaults.swift:74:7:74:7 | self | file://:0:0:0:0 | .value | testUserDefaults.swift:82:28:82:40 | .value |
3848
#select
3949
| testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | This operation stores 'password' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | password |
4050
| testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x | testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | This operation stores 'x' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x | x |
@@ -50,3 +60,4 @@ subpaths
5060
| testUserDefaults.swift:59:28:59:28 | x | testUserDefaults.swift:55:10:55:10 | passwd | testUserDefaults.swift:59:28:59:28 | x | This operation stores 'x' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:55:10:55:10 | passwd | passwd |
5161
| testUserDefaults.swift:60:28:60:28 | y | testUserDefaults.swift:56:10:56:10 | passwd | testUserDefaults.swift:60:28:60:28 | y | This operation stores 'y' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:56:10:56:10 | passwd | passwd |
5262
| testUserDefaults.swift:61:28:61:28 | z | testUserDefaults.swift:57:10:57:10 | passwd | testUserDefaults.swift:61:28:61:28 | z | This operation stores 'z' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:57:10:57:10 | passwd | passwd |
63+
| testUserDefaults.swift:82:28:82:40 | .value | testUserDefaults.swift:82:28:82:31 | .password | testUserDefaults.swift:82:28:82:40 | .value | This operation stores '.value' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:82:28:82:31 | .password | .password |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,6 @@ struct MyOuter {
170170
}
171171

172172
func test3(mo : MyOuter) {
173-
NSLog(mo.password.value) // BAD [NOT DETECTED]
174-
NSLog(mo.harmless.value) // GOOD
173+
NSLog(mo.password.value) // $ hasCleartextLogging=173
174+
NSLog(mo.harmless.value) // Safe
175175
}

‎swift/ql/test/query-tests/Security/CWE-312/testUserDefaults.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,6 @@ struct MyOuter {
7979
}
8080

8181
func test5(mo : MyOuter) {
82-
UserDefaults.standard.set(mo.password.value, forKey: "myKey") // BAD [NOT DETECTED]
82+
UserDefaults.standard.set(mo.password.value, forKey: "myKey") // BAD
8383
UserDefaults.standard.set(mo.harmless.value, forKey: "myKey") // GOOD
8484
}

0 commit comments

Comments
 (0)