Skip to content

Commit a0cba8c

Browse files
geoffw0geoffw0
committed
Swift: Address boolean value FPs.
1 parent 27c8eb3 commit a0cba8c

File tree

6 files changed

+37
-13
lines changed

6 files changed

+37
-13
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ private class DefaultCleartextLoggingSink extends CleartextLoggingSink {
2929
DefaultCleartextLoggingSink() { sinkNode(this, "log-injection") }
3030
}
3131

32+
/**
33+
* An barrier for cleartext logging vulnerabilities.
34+
* - encryption; encrypted values are not cleartext.
35+
* - booleans; these are more likely to be settings, rather than actual sensitive data.
36+
*/
37+
private class CleartextLoggingDefaultBarrier extends CleartextLoggingBarrier {
38+
CleartextLoggingDefaultBarrier() {
39+
this.asExpr() instanceof EncryptedExpr or
40+
this.asExpr().getType().getUnderlyingType() instanceof BoolType
41+
}
42+
}
43+
3244
/**
3345
* A barrier for `OSLogMessage`s configured with the appropriate privacy option.
3446
* Numeric and boolean arguments aren't redacted unless the `private` or `sensitive` options are used.

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,15 @@ private class CleartextStorageDatabaseSinks extends SinkModelCsv {
114114
}
115115

116116
/**
117-
* An encryption barrier for cleartext database storage vulnerabilities.
117+
* An barrier for cleartext database storage vulnerabilities.
118+
* - encryption; encrypted values are not cleartext.
119+
* - booleans; these are more likely to be settings, rather than actual sensitive data.
118120
*/
119-
private class CleartextStorageDatabaseEncryptionBarrier extends CleartextStorageDatabaseBarrier {
120-
CleartextStorageDatabaseEncryptionBarrier() { this.asExpr() instanceof EncryptedExpr }
121+
private class CleartextStorageDatabaseDefaultBarrier extends CleartextStorageDatabaseBarrier {
122+
CleartextStorageDatabaseDefaultBarrier() {
123+
this.asExpr() instanceof EncryptedExpr or
124+
this.asExpr().getType().getUnderlyingType() instanceof BoolType
125+
}
121126
}
122127

123128
/**

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,15 @@ private class NSUserDefaultsControllerStore extends CleartextStoragePreferencesS
7474
}
7575

7676
/**
77-
* An encryption barrier for cleartext preferences storage vulnerabilities.
77+
* An barrier for cleartext preferences storage vulnerabilities.
78+
* - encryption; encrypted values are not cleartext.
79+
* - booleans; these are more likely to be settings, rather than actual sensitive data.
7880
*/
79-
private class CleartextStoragePreferencesEncryptionBarrier extends CleartextStoragePreferencesBarrier
80-
{
81-
CleartextStoragePreferencesEncryptionBarrier() { this.asExpr() instanceof EncryptedExpr }
81+
private class CleartextStoragePreferencesDefaultBarrier extends CleartextStoragePreferencesBarrier {
82+
CleartextStoragePreferencesDefaultBarrier() {
83+
this.asExpr() instanceof EncryptedExpr or
84+
this.asExpr().getType().getUnderlyingType() instanceof BoolType
85+
}
8286
}
8387

8488
/**

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,15 @@ private class AlamofireTransmittedSink extends CleartextTransmissionSink {
8181
}
8282

8383
/**
84-
* An encryption barrier for cleartext transmission vulnerabilities.
84+
* An barrier for cleartext transmission vulnerabilities.
85+
* - encryption; encrypted values are not cleartext.
86+
* - booleans; these are more likely to be settings, rather than actual sensitive data.
8587
*/
86-
private class CleartextTransmissionEncryptionBarrier extends CleartextTransmissionBarrier {
87-
CleartextTransmissionEncryptionBarrier() { this.asExpr() instanceof EncryptedExpr }
88+
private class CleartextTransmissionDefaultBarrier extends CleartextTransmissionBarrier {
89+
CleartextTransmissionDefaultBarrier() {
90+
this.asExpr() instanceof EncryptedExpr or
91+
this.asExpr().getType().getUnderlyingType() instanceof BoolType
92+
}
8893
}
8994

9095
/**

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ nodes
3939
| testSend.swift:65:27:65:27 | license_key | semmle.label | license_key |
4040
| testSend.swift:66:27:66:30 | .mobileNumber | semmle.label | .mobileNumber |
4141
| testSend.swift:68:27:68:30 | .mobilePlayer | semmle.label | .mobilePlayer |
42-
| testSend.swift:69:27:69:30 | .passwordFeatureEnabled | semmle.label | .passwordFeatureEnabled |
4342
| testURL.swift:13:22:13:54 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
4443
| testURL.swift:13:54:13:54 | passwd | semmle.label | passwd |
4544
| testURL.swift:16:22:16:55 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
@@ -60,7 +59,6 @@ subpaths
6059
| testSend.swift:65:27:65:27 | license_key | testSend.swift:65:27:65:27 | license_key | testSend.swift:65:27:65:27 | license_key | This operation transmits 'license_key', which may contain unencrypted sensitive data from $@. | testSend.swift:65:27:65:27 | license_key | license_key |
6160
| testSend.swift:66:27:66:30 | .mobileNumber | testSend.swift:66:27:66:30 | .mobileNumber | testSend.swift:66:27:66:30 | .mobileNumber | This operation transmits '.mobileNumber', which may contain unencrypted sensitive data from $@. | testSend.swift:66:27:66:30 | .mobileNumber | .mobileNumber |
6261
| testSend.swift:68:27:68:30 | .mobilePlayer | testSend.swift:68:27:68:30 | .mobilePlayer | testSend.swift:68:27:68:30 | .mobilePlayer | This operation transmits '.mobilePlayer', which may contain unencrypted sensitive data from $@. | testSend.swift:68:27:68:30 | .mobilePlayer | .mobilePlayer |
63-
| testSend.swift:69:27:69:30 | .passwordFeatureEnabled | testSend.swift:69:27:69:30 | .passwordFeatureEnabled | testSend.swift:69:27:69:30 | .passwordFeatureEnabled | This operation transmits '.passwordFeatureEnabled', which may contain unencrypted sensitive data from $@. | testSend.swift:69:27:69:30 | .passwordFeatureEnabled | .passwordFeatureEnabled |
6462
| testURL.swift:13:22:13:54 | ... .+(_:_:) ... | testURL.swift:13:54:13:54 | passwd | testURL.swift:13:22:13:54 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:13:54:13:54 | passwd | passwd |
6563
| testURL.swift:16:22:16:55 | ... .+(_:_:) ... | testURL.swift:16:55:16:55 | credit_card_no | testURL.swift:16:22:16:55 | ... .+(_:_:) ... | This operation transmits '... .+(_:_:) ...', which may contain unencrypted sensitive data from $@. | testURL.swift:16:55:16:55 | credit_card_no | credit_card_no |
6664
| testURL.swift:20:22:20:22 | passwd | testURL.swift:20:22:20:22 | passwd | testURL.swift:20:22:20:22 | passwd | This operation transmits 'passwd', which may contain unencrypted sensitive data from $@. | testURL.swift:20:22:20:22 | passwd | passwd |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,5 @@ func test2(password : String, license_key: String, ms: MyStruct, connection : NW
6666
connection.send(content: ms.mobileNumber, completion: .idempotent) // BAD
6767
connection.send(content: ms.mobileUrl, completion: .idempotent) // GOOD (not sensitive)
6868
connection.send(content: ms.mobilePlayer, completion: .idempotent) // GOOD (not sensitive) [FALSE POSITIVE]
69-
connection.send(content: ms.passwordFeatureEnabled, completion: .idempotent) // GOOD (not sensitive) [FALSE POSITIVE]
69+
connection.send(content: ms.passwordFeatureEnabled, completion: .idempotent) // GOOD (not sensitive)
7070
}

0 commit comments

Comments
 (0)