Skip to content

Commit c739bbb

Browse files
redsun82redsun82
committed
Swift: bake in isProbablySafe in SensitiveExpr
Also restructured the code a bit in the weak hashing query.
1 parent a5233c0 commit c739bbb

File tree

7 files changed

+50
-70
lines changed

7 files changed

+50
-70
lines changed

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

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -121,20 +121,24 @@ class SensitiveExpr extends Expr {
121121
SensitiveDataType sensitiveType;
122122

123123
SensitiveExpr() {
124-
// variable reference
125-
this.(DeclRefExpr).getDecl().(SensitiveVarDecl).hasInfo(label, sensitiveType)
126-
or
127-
// member variable reference
128-
this.(MemberRefExpr).getMember().(SensitiveVarDecl).hasInfo(label, sensitiveType)
129-
or
130-
// function call
131-
this.(ApplyExpr).getStaticTarget().(SensitiveFunctionDecl).hasInfo(label, sensitiveType)
132-
or
133-
// sensitive argument
134-
exists(SensitiveArgument a |
135-
a.hasInfo(label, sensitiveType) and
136-
a.getExpr() = this
137-
)
124+
(
125+
// variable reference
126+
this.(DeclRefExpr).getDecl().(SensitiveVarDecl).hasInfo(label, sensitiveType)
127+
or
128+
// member variable reference
129+
this.(MemberRefExpr).getMember().(SensitiveVarDecl).hasInfo(label, sensitiveType)
130+
or
131+
// function call
132+
this.(ApplyExpr).getStaticTarget().(SensitiveFunctionDecl).hasInfo(label, sensitiveType)
133+
or
134+
// sensitive argument
135+
exists(SensitiveArgument a |
136+
a.hasInfo(label, sensitiveType) and
137+
a.getExpr() = this
138+
)
139+
) and
140+
// do not mark as sensitive it if it is probably safe
141+
not label.toLowerCase().regexpMatch(regexpProbablySafe())
138142
}
139143

140144
/**
@@ -146,11 +150,4 @@ class SensitiveExpr extends Expr {
146150
* Gets the type of sensitive expression this is.
147151
*/
148152
SensitiveDataType getSensitiveType() { result = sensitiveType }
149-
150-
/**
151-
* Holds if this sensitive expression might be safe because it contains
152-
* hashed or encrypted data, or is only a reference to data that is stored
153-
* elsewhere.
154-
*/
155-
predicate isProbablySafe() { label.toLowerCase().regexpMatch(regexpProbablySafe()) }
156153
}

‎swift/ql/src/queries/Security/CWE-311/CleartextStorageDatabase.ql

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,7 @@ class RealmStore extends Stored {
7070
class CleartextStorageConfig extends TaintTracking::Configuration {
7171
CleartextStorageConfig() { this = "CleartextStorageConfig" }
7272

73-
override predicate isSource(DataFlow::Node node) {
74-
exists(SensitiveExpr e |
75-
node.asExpr() = e and
76-
not e.isProbablySafe()
77-
)
78-
}
73+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
7974

8075
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof Stored }
8176

‎swift/ql/src/queries/Security/CWE-311/CleartextTransmission.ql

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,7 @@ class URL extends Transmitted {
6363
class CleartextTransmissionConfig extends TaintTracking::Configuration {
6464
CleartextTransmissionConfig() { this = "CleartextTransmissionConfig" }
6565

66-
override predicate isSource(DataFlow::Node node) {
67-
exists(SensitiveExpr e |
68-
node.asExpr() = e and
69-
not e.isProbablySafe()
70-
)
71-
}
66+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
7267

7368
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof Transmitted }
7469

‎swift/ql/src/queries/Security/CWE-328/WeakSensitiveDataHashing.ql

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,24 @@ import DataFlow::PathGraph
2020
class WeakHashingConfig extends TaintTracking::Configuration {
2121
WeakHashingConfig() { this = "WeakHashingConfig" }
2222

23-
override predicate isSource(DataFlow::Node node) {
24-
exists(SensitiveExpr e |
25-
node.asExpr() = e and
26-
not e.isProbablySafe()
27-
)
28-
}
23+
override predicate isSource(DataFlow::Node node) { node instanceof WeakHashingConfig::Source }
2924

3025
override predicate isSink(DataFlow::Node node) { node instanceof WeakHashingConfig::Sink }
3126
}
3227

3328
module WeakHashingConfig {
34-
class Sink extends DataFlow::Node {
29+
class Source extends DataFlow::Node {
30+
Source() { this.asExpr() instanceof SensitiveExpr }
31+
}
32+
33+
abstract class Sink extends DataFlow::Node {
34+
abstract string getAlgorithm();
35+
}
36+
37+
class CryptoHash extends Sink {
3538
string algorithm;
3639

37-
Sink() {
40+
CryptoHash() {
3841
exists(ApplyExpr call, FuncDecl func |
3942
call.getAnArgument().getExpr() = this.asExpr() and
4043
call.getStaticTarget() = func and
@@ -44,14 +47,17 @@ module WeakHashingConfig {
4447
)
4548
}
4649

47-
string getAlgorithm() { result = algorithm }
50+
override string getAlgorithm() { result = algorithm }
4851
}
4952
}
5053

51-
from WeakHashingConfig config, DataFlow::PathNode source, DataFlow::PathNode sink, string algorithm
54+
from
55+
WeakHashingConfig config, DataFlow::PathNode source, DataFlow::PathNode sink, string algorithm,
56+
SensitiveExpr expr
5257
where
5358
config.hasFlowPath(source, sink) and
54-
algorithm = sink.getNode().(WeakHashingConfig::Sink).getAlgorithm()
59+
algorithm = sink.getNode().(WeakHashingConfig::Sink).getAlgorithm() and
60+
expr = source.getNode().asExpr()
5561
select sink.getNode(), source, sink,
5662
"Insecure hashing algorithm (" + algorithm + ") depends on $@.", source.getNode(),
57-
"sensitive data"
63+
"sensitive data (" + expr.getSensitiveType() + " " + expr.getLabel() + ")"

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
| testCoreData.swift:48:15:48:15 | password | label:password, type:credential |
2-
| testCoreData.swift:49:15:49:15 | password_hash | isProbablySafe, label:password_hash, type:credential |
32
| testCoreData.swift:51:24:51:24 | password | label:password, type:credential |
4-
| testCoreData.swift:52:24:52:24 | password_hash | isProbablySafe, label:password_hash, type:credential |
53
| testCoreData.swift:58:15:58:15 | password | label:password, type:credential |
6-
| testCoreData.swift:59:15:59:15 | password_file | isProbablySafe, label:password_file, type:credential |
74
| testCoreData.swift:61:25:61:25 | password | label:password, type:credential |
8-
| testCoreData.swift:62:25:62:25 | password_file | isProbablySafe, label:password_file, type:credential |
95
| testCoreData.swift:64:16:64:16 | password | label:password, type:credential |
10-
| testCoreData.swift:65:16:65:16 | password_file | isProbablySafe, label:password_file, type:credential |
116
| testCoreData.swift:77:2:77:25 | call to doSomething(password:) | label:doSomething(password:), type:credential |
127
| testCoreData.swift:77:24:77:24 | x | label:password, type:credential |
138
| testCoreData.swift:80:10:80:22 | call to getPassword() | label:getPassword(), type:credential |
@@ -16,16 +11,10 @@
1611
| testCoreData.swift:92:10:92:10 | passwd | label:passwd, type:credential |
1712
| testCoreData.swift:93:10:93:10 | passwd | label:passwd, type:credential |
1813
| testRealm.swift:34:11:34:11 | myPassword | label:myPassword, type:credential |
19-
| testRealm.swift:38:11:38:11 | myHashedPassword | isProbablySafe, label:myHashedPassword, type:credential |
2014
| testRealm.swift:42:11:42:11 | myPassword | label:myPassword, type:credential |
21-
| testRealm.swift:46:11:46:11 | myHashedPassword | isProbablySafe, label:myHashedPassword, type:credential |
2215
| testRealm.swift:52:12:52:12 | myPassword | label:myPassword, type:credential |
23-
| testRealm.swift:55:12:55:12 | myHashedPassword | isProbablySafe, label:myHashedPassword, type:credential |
2416
| testSend.swift:29:19:29:19 | passwordPlain | label:passwordPlain, type:credential |
25-
| testSend.swift:30:19:30:19 | passwordHash | isProbablySafe, label:passwordHash, type:credential |
2617
| testSend.swift:33:19:33:19 | passwordPlain | label:passwordPlain, type:credential |
27-
| testSend.swift:34:19:34:19 | passwordHash | isProbablySafe, label:passwordHash, type:credential |
2818
| testURL.swift:13:54:13:54 | passwd | label:passwd, type:credential |
29-
| testURL.swift:14:54:14:54 | encrypted_passwd | isProbablySafe, label:encrypted_passwd, type:credential |
3019
| testURL.swift:16:55:16:55 | credit_card_no | label:credit_card_no, type:private information |
3120
| testURL.swift:20:22:20:22 | passwd | label:passwd, type:credential |

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ string describe(SensitiveExpr e) {
55
result = "label:" + e.getLabel()
66
or
77
result = "type:" + e.getSensitiveType().toString()
8-
or
9-
e.isProbablySafe() and result = "isProbablySafe"
108
}
119

1210
from SensitiveExpr e

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ nodes
1414
| testCrypto.swift:67:32:67:32 | credit_card_no | semmle.label | credit_card_no |
1515
subpaths
1616
#select
17-
| testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:25:47:25:47 | passwd | sensitive data |
18-
| testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:28:43:28:43 | credit_card_no | sensitive data |
19-
| testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:32:48:32:48 | passwd | sensitive data |
20-
| testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:35:44:35:44 | credit_card_no | sensitive data |
21-
| testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:40:23:40:23 | passwd | sensitive data |
22-
| testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:43:23:43:23 | credit_card_no | sensitive data |
23-
| testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:48:23:48:23 | passwd | sensitive data |
24-
| testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:51:23:51:23 | credit_card_no | sensitive data |
25-
| testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:56:32:56:32 | passwd | sensitive data |
26-
| testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:59:32:59:32 | credit_card_no | sensitive data |
27-
| testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:64:32:64:32 | passwd | sensitive data |
28-
| testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:67:32:67:32 | credit_card_no | sensitive data |
17+
| testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:25:47:25:47 | passwd | sensitive data (credential passwd) |
18+
| testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:28:43:28:43 | credit_card_no | sensitive data (private information credit_card_no) |
19+
| testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:32:48:32:48 | passwd | sensitive data (credential passwd) |
20+
| testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:35:44:35:44 | credit_card_no | sensitive data (private information credit_card_no) |
21+
| testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:40:23:40:23 | passwd | sensitive data (credential passwd) |
22+
| testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:43:23:43:23 | credit_card_no | sensitive data (private information credit_card_no) |
23+
| testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:48:23:48:23 | passwd | sensitive data (credential passwd) |
24+
| testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:51:23:51:23 | credit_card_no | sensitive data (private information credit_card_no) |
25+
| testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:56:32:56:32 | passwd | sensitive data (credential passwd) |
26+
| testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:59:32:59:32 | credit_card_no | sensitive data (private information credit_card_no) |
27+
| testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:64:32:64:32 | passwd | sensitive data (credential passwd) |
28+
| testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:67:32:67:32 | credit_card_no | sensitive data (private information credit_card_no) |

0 commit comments

Comments
 (0)