Skip to content

Commit 94e6ec6

Browse files
committed
Swift: Widen the new sinks to cover more cases the MaD sinks are missing.
1 parent c902c75 commit 94e6ec6

5 files changed

Lines changed: 35 additions & 13 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ private class WeakPasswordHashingMetatypeSink extends WeakPasswordHashingSink {
121121
exists(CallExpr c |
122122
c.getAnArgument().getExpr() = this.asExpr() and
123123
algorithm = ["SHA256", "SHA384", "SHA512"] and
124-
c.getQualifier().getType().getFullName() = algorithm + ".Type" and
124+
c.getQualifier().getType().getFullName() = algorithm + ["", ".Type"] and
125125
c.getStaticTarget().getName() = ["hash(data:)", "update(data:)", "update(bufferPointer:)"]
126126
)
127127
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ private class WeakSenitiveDataHashingMetatypeSink extends WeakSensitiveDataHashi
8787
exists(CallExpr c |
8888
c.getAnArgument().getExpr() = this.asExpr() and
8989
algorithm = ["MD5", "SHA1"] and
90-
c.getQualifier().getType().getFullName() = "Insecure." + algorithm + ".Type" and
90+
c.getQualifier().getType().getFullName() = "Insecure." + algorithm + ["", ".Type"] and
9191
c.getStaticTarget().getName() = ["hash(data:)", "update(data:)", "update(bufferPointer:)"]
9292
)
9393
}

swift/ql/test/query-tests/Security/CWE-328/WeakPasswordHashing.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ nodes
88
| testCryptoKit.swift:102:37:102:37 | passwd | semmle.label | passwd |
99
| testCryptoKit.swift:108:37:108:37 | passwd | semmle.label | passwd |
1010
| testCryptoKit.swift:114:37:114:37 | passwd | semmle.label | passwd |
11+
| testCryptoKit.swift:123:23:123:23 | passwd | semmle.label | passwd |
12+
| testCryptoKit.swift:132:23:132:23 | passwd | semmle.label | passwd |
13+
| testCryptoKit.swift:141:23:141:23 | passwd | semmle.label | passwd |
14+
| testCryptoKit.swift:150:23:150:23 | passwd | semmle.label | passwd |
15+
| testCryptoKit.swift:159:23:159:23 | passwd | semmle.label | passwd |
1116
| testCryptoKit.swift:168:32:168:32 | passwd | semmle.label | passwd |
1217
| testCryptoKit.swift:177:32:177:32 | passwd | semmle.label | passwd |
1318
| testCryptoKit.swift:186:32:186:32 | passwd | semmle.label | passwd |
@@ -49,6 +54,11 @@ subpaths
4954
| testCryptoKit.swift:102:37:102:37 | passwd | testCryptoKit.swift:102:37:102:37 | passwd | testCryptoKit.swift:102:37:102:37 | passwd | Insecure hashing algorithm (SHA256) depends on $@. | testCryptoKit.swift:102:37:102:37 | passwd | password (passwd) |
5055
| testCryptoKit.swift:108:37:108:37 | passwd | testCryptoKit.swift:108:37:108:37 | passwd | testCryptoKit.swift:108:37:108:37 | passwd | Insecure hashing algorithm (SHA384) depends on $@. | testCryptoKit.swift:108:37:108:37 | passwd | password (passwd) |
5156
| testCryptoKit.swift:114:37:114:37 | passwd | testCryptoKit.swift:114:37:114:37 | passwd | testCryptoKit.swift:114:37:114:37 | passwd | Insecure hashing algorithm (SHA512) depends on $@. | testCryptoKit.swift:114:37:114:37 | passwd | password (passwd) |
57+
| testCryptoKit.swift:123:23:123:23 | passwd | testCryptoKit.swift:123:23:123:23 | passwd | testCryptoKit.swift:123:23:123:23 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:123:23:123:23 | passwd | password (passwd) |
58+
| testCryptoKit.swift:132:23:132:23 | passwd | testCryptoKit.swift:132:23:132:23 | passwd | testCryptoKit.swift:132:23:132:23 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:132:23:132:23 | passwd | password (passwd) |
59+
| testCryptoKit.swift:141:23:141:23 | passwd | testCryptoKit.swift:141:23:141:23 | passwd | testCryptoKit.swift:141:23:141:23 | passwd | Insecure hashing algorithm (SHA256) depends on $@. | testCryptoKit.swift:141:23:141:23 | passwd | password (passwd) |
60+
| testCryptoKit.swift:150:23:150:23 | passwd | testCryptoKit.swift:150:23:150:23 | passwd | testCryptoKit.swift:150:23:150:23 | passwd | Insecure hashing algorithm (SHA384) depends on $@. | testCryptoKit.swift:150:23:150:23 | passwd | password (passwd) |
61+
| testCryptoKit.swift:159:23:159:23 | passwd | testCryptoKit.swift:159:23:159:23 | passwd | testCryptoKit.swift:159:23:159:23 | passwd | Insecure hashing algorithm (SHA512) depends on $@. | testCryptoKit.swift:159:23:159:23 | passwd | password (passwd) |
5262
| testCryptoKit.swift:168:32:168:32 | passwd | testCryptoKit.swift:168:32:168:32 | passwd | testCryptoKit.swift:168:32:168:32 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:168:32:168:32 | passwd | password (passwd) |
5363
| testCryptoKit.swift:177:32:177:32 | passwd | testCryptoKit.swift:177:32:177:32 | passwd | testCryptoKit.swift:177:32:177:32 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:177:32:177:32 | passwd | password (passwd) |
5464
| testCryptoKit.swift:186:32:186:32 | passwd | testCryptoKit.swift:186:32:186:32 | passwd | testCryptoKit.swift:186:32:186:32 | passwd | Insecure hashing algorithm (SHA256) depends on $@. | testCryptoKit.swift:186:32:186:32 | passwd | password (passwd) |

swift/ql/test/query-tests/Security/CWE-328/WeakSensitiveDataHashing.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ nodes
2828
| testCryptoKit.swift:97:44:97:44 | cert | semmle.label | cert |
2929
| testCryptoKit.swift:99:44:99:44 | account_no | semmle.label | account_no |
3030
| testCryptoKit.swift:100:44:100:44 | credit_card_no | semmle.label | credit_card_no |
31+
| testCryptoKit.swift:124:23:124:23 | cert | semmle.label | cert |
32+
| testCryptoKit.swift:126:23:126:23 | account_no | semmle.label | account_no |
33+
| testCryptoKit.swift:127:23:127:23 | credit_card_no | semmle.label | credit_card_no |
34+
| testCryptoKit.swift:133:23:133:23 | cert | semmle.label | cert |
35+
| testCryptoKit.swift:135:23:135:23 | account_no | semmle.label | account_no |
36+
| testCryptoKit.swift:136:23:136:23 | credit_card_no | semmle.label | credit_card_no |
3137
| testCryptoKit.swift:169:32:169:32 | cert | semmle.label | cert |
3238
| testCryptoKit.swift:171:32:171:32 | account_no | semmle.label | account_no |
3339
| testCryptoKit.swift:172:32:172:32 | credit_card_no | semmle.label | credit_card_no |
@@ -79,6 +85,12 @@ subpaths
7985
| testCryptoKit.swift:97:44:97:44 | cert | testCryptoKit.swift:97:44:97:44 | cert | testCryptoKit.swift:97:44:97:44 | cert | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:97:44:97:44 | cert | sensitive data (credential cert) |
8086
| testCryptoKit.swift:99:44:99:44 | account_no | testCryptoKit.swift:99:44:99:44 | account_no | testCryptoKit.swift:99:44:99:44 | account_no | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:99:44:99:44 | account_no | sensitive data (private information account_no) |
8187
| testCryptoKit.swift:100:44:100:44 | credit_card_no | testCryptoKit.swift:100:44:100:44 | credit_card_no | testCryptoKit.swift:100:44:100:44 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:100:44:100:44 | credit_card_no | sensitive data (private information credit_card_no) |
88+
| testCryptoKit.swift:124:23:124:23 | cert | testCryptoKit.swift:124:23:124:23 | cert | testCryptoKit.swift:124:23:124:23 | cert | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:124:23:124:23 | cert | sensitive data (credential cert) |
89+
| testCryptoKit.swift:126:23:126:23 | account_no | testCryptoKit.swift:126:23:126:23 | account_no | testCryptoKit.swift:126:23:126:23 | account_no | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:126:23:126:23 | account_no | sensitive data (private information account_no) |
90+
| testCryptoKit.swift:127:23:127:23 | credit_card_no | testCryptoKit.swift:127:23:127:23 | credit_card_no | testCryptoKit.swift:127:23:127:23 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:127:23:127:23 | credit_card_no | sensitive data (private information credit_card_no) |
91+
| testCryptoKit.swift:133:23:133:23 | cert | testCryptoKit.swift:133:23:133:23 | cert | testCryptoKit.swift:133:23:133:23 | cert | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:133:23:133:23 | cert | sensitive data (credential cert) |
92+
| testCryptoKit.swift:135:23:135:23 | account_no | testCryptoKit.swift:135:23:135:23 | account_no | testCryptoKit.swift:135:23:135:23 | account_no | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:135:23:135:23 | account_no | sensitive data (private information account_no) |
93+
| testCryptoKit.swift:136:23:136:23 | credit_card_no | testCryptoKit.swift:136:23:136:23 | credit_card_no | testCryptoKit.swift:136:23:136:23 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCryptoKit.swift:136:23:136:23 | credit_card_no | sensitive data (private information credit_card_no) |
8294
| testCryptoKit.swift:169:32:169:32 | cert | testCryptoKit.swift:169:32:169:32 | cert | testCryptoKit.swift:169:32:169:32 | cert | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:169:32:169:32 | cert | sensitive data (credential cert) |
8395
| testCryptoKit.swift:171:32:171:32 | account_no | testCryptoKit.swift:171:32:171:32 | account_no | testCryptoKit.swift:171:32:171:32 | account_no | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:171:32:171:32 | account_no | sensitive data (private information account_no) |
8496
| testCryptoKit.swift:172:32:172:32 | credit_card_no | testCryptoKit.swift:172:32:172:32 | credit_card_no | testCryptoKit.swift:172:32:172:32 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCryptoKit.swift:172:32:172:32 | credit_card_no | sensitive data (private information credit_card_no) |

swift/ql/test/query-tests/Security/CWE-328/testCryptoKit.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,25 +120,25 @@ func testHashMethods(passwd : UnsafeRawBufferPointer, cert: String, encrypted_pa
120120

121121
func testMD5UpdateWithData(passwd : String, cert: String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
122122
var hash = Crypto.Insecure.MD5()
123-
hash.update(data: passwd) // BAD [NOT DETECTED]
124-
hash.update(data: cert) // BAD [NOT DETECTED]
123+
hash.update(data: passwd) // BAD
124+
hash.update(data: cert) // BAD
125125
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
126-
hash.update(data: account_no) // BAD [NOT DETECTED]
127-
hash.update(data: credit_card_no) // BAD [NOT DETECTED]
126+
hash.update(data: account_no) // BAD
127+
hash.update(data: credit_card_no) // BAD
128128
}
129129

130130
func testSHA1UpdateWithData(passwd : String, cert: String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
131131
var hash = Crypto.Insecure.SHA1()
132-
hash.update(data: passwd) // BAD [NOT DETECTED]
133-
hash.update(data: cert) // BAD [NOT DETECTED]
132+
hash.update(data: passwd) // BAD
133+
hash.update(data: cert) // BAD
134134
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
135-
hash.update(data: account_no) // BAD [NOT DETECTED]
136-
hash.update(data: credit_card_no) // BAD [NOT DETECTED]
135+
hash.update(data: account_no) // BAD
136+
hash.update(data: credit_card_no) // BAD
137137
}
138138

139139
func testSHA256UpdateWithData(passwd : String, cert: String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
140140
var hash = Crypto.SHA256()
141-
hash.update(data: passwd) // BAD, not a computationally expensive hash [NOT DETECTED]
141+
hash.update(data: passwd) // BAD, not a computationally expensive hash
142142
hash.update(data: cert) // GOOD
143143
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
144144
hash.update(data: account_no) // GOOD
@@ -147,7 +147,7 @@ func testSHA256UpdateWithData(passwd : String, cert: String, encrypted_passwd :
147147

148148
func testSHA384UpdateWithData(passwd : String, cert: String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
149149
var hash = Crypto.SHA384()
150-
hash.update(data: passwd) // BAD, not a computationally expensive hash [NOT DETECTED]
150+
hash.update(data: passwd) // BAD, not a computationally expensive hash
151151
hash.update(data: cert) // GOOD
152152
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
153153
hash.update(data: account_no) // GOOD
@@ -156,7 +156,7 @@ func testSHA384UpdateWithData(passwd : String, cert: String, encrypted_passwd :
156156

157157
func testSHA512UpdateWithData(passwd : String, cert: String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
158158
var hash = Crypto.SHA512()
159-
hash.update(data: passwd) // BAD, not a computationally expensive hash [NOT DETECTED]
159+
hash.update(data: passwd) // BAD, not a computationally expensive hash
160160
hash.update(data: cert) // GOOD
161161
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
162162
hash.update(data: account_no) // GOOD

0 commit comments

Comments
 (0)