Skip to content

Commit 3f8ac1a

Browse files
authored
Merge pull request github#12794 from geoffw0/modernsec2
Swift: Add CSV extension points to the encryption queries.
2 parents 0442072 + 8c415f3 commit 3f8ac1a

8 files changed

+117
-56
lines changed

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow sink for constant password vulnerabilities. That is,
@@ -31,10 +32,10 @@ class ConstantPasswordAdditionalTaintStep extends Unit {
3132
/**
3233
* A password sink for the CryptoSwift library.
3334
*/
34-
private class DefaultConstantPasswordSink extends ConstantPasswordSink {
35-
DefaultConstantPasswordSink() {
35+
private class CryptoSwiftPasswordSink extends ConstantPasswordSink {
36+
CryptoSwiftPasswordSink() {
3637
// `password` arg in `init` is a sink
37-
exists(ClassOrStructDecl c, ConstructorDecl f, CallExpr call |
38+
exists(NominalTypeDecl c, ConstructorDecl f, CallExpr call |
3839
c.getName() = ["HKDF", "PBKDF1", "PBKDF2", "Scrypt"] and
3940
c.getAMember() = f and
4041
call.getStaticTarget() = f and
@@ -49,8 +50,12 @@ private class DefaultConstantPasswordSink extends ConstantPasswordSink {
4950
private class RnCryptorPasswordSink extends ConstantPasswordSink {
5051
RnCryptorPasswordSink() {
5152
// RNCryptor (labelled arguments)
52-
exists(ClassOrStructDecl c, MethodDecl f, CallExpr call |
53-
c.getName() = ["RNCryptor", "RNEncryptor", "RNDecryptor"] and
53+
exists(NominalTypeDecl c, MethodDecl f, CallExpr call |
54+
c.getFullName() =
55+
[
56+
"RNCryptor", "RNEncryptor", "RNDecryptor", "RNCryptor.EncryptorV3",
57+
"RNCryptor.DecryptorV3"
58+
] and
5459
c.getAMember() = f and
5560
call.getStaticTarget() = f and
5661
call.getArgumentWithLabel(["password", "withPassword", "forPassword"]).getExpr() =
@@ -65,3 +70,10 @@ private class RnCryptorPasswordSink extends ConstantPasswordSink {
6570
)
6671
}
6772
}
73+
74+
/**
75+
* A sink defined in a CSV model.
76+
*/
77+
private class DefaultPasswordSink extends ConstantPasswordSink {
78+
DefaultPasswordSink() { sinkNode(this, "encryption-password") }
79+
}

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow sink for constant salt vulnerabilities. That is,
@@ -34,7 +35,7 @@ class ConstantSaltAdditionalTaintStep extends Unit {
3435
private class CryptoSwiftSaltSink extends ConstantSaltSink {
3536
CryptoSwiftSaltSink() {
3637
// `salt` arg in `init` is a sink
37-
exists(ClassOrStructDecl c, ConstructorDecl f, CallExpr call |
38+
exists(NominalTypeDecl c, ConstructorDecl f, CallExpr call |
3839
c.getName() = ["HKDF", "PBKDF1", "PBKDF2", "Scrypt"] and
3940
c.getAMember() = f and
4041
call.getStaticTarget() = f and
@@ -48,12 +49,23 @@ private class CryptoSwiftSaltSink extends ConstantSaltSink {
4849
*/
4950
private class RnCryptorSaltSink extends ConstantSaltSink {
5051
RnCryptorSaltSink() {
51-
exists(ClassOrStructDecl c, MethodDecl f, CallExpr call |
52-
c.getName() = ["RNCryptor", "RNEncryptor", "RNDecryptor"] and
52+
exists(NominalTypeDecl c, MethodDecl f, CallExpr call |
53+
c.getFullName() =
54+
[
55+
"RNCryptor", "RNEncryptor", "RNDecryptor", "RNCryptor.EncryptorV3",
56+
"RNCryptor.DecryptorV3"
57+
] and
5358
c.getAMember() = f and
5459
call.getStaticTarget() = f and
5560
call.getArgumentWithLabel(["salt", "encryptionSalt", "hmacSalt", "HMACSalt"]).getExpr() =
5661
this.asExpr()
5762
)
5863
}
5964
}
65+
66+
/**
67+
* A sink defined in a CSV model.
68+
*/
69+
private class DefaultSaltSink extends ConstantSaltSink {
70+
DefaultSaltSink() { sinkNode(this, "encryption-salt") }
71+
}

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

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow source for ECB encryption vulnerabilities. That is,
@@ -48,32 +49,22 @@ private class CryptoSwiftEcb extends EcbEncryptionSource {
4849
}
4950
}
5051

51-
/**
52-
* A block mode being used to form a CryptoSwift `AES` cipher.
53-
*/
54-
private class AES extends EcbEncryptionSink {
55-
AES() {
56-
// `blockMode` arg in `AES.init` is a sink
57-
exists(CallExpr call |
58-
call.getStaticTarget()
59-
.(MethodDecl)
60-
.hasQualifiedName("AES", ["init(key:blockMode:)", "init(key:blockMode:padding:)"]) and
61-
call.getArgument(1).getExpr() = this.asExpr()
62-
)
52+
private class EcbEncryptionSinks extends SinkModelCsv {
53+
override predicate row(string row) {
54+
row =
55+
[
56+
// CryptoSwift `AES.init` block mode
57+
";AES;true;init(key:blockMode:);;;Argument[1];encryption-block-mode",
58+
";AES;true;init(key:blockMode:padding:);;;Argument[1];encryption-block-mode",
59+
// CryptoSwift `Blowfish.init` block mode
60+
";Blowfish;true;init(key:blockMode:padding:);;;Argument[1];encryption-block-mode",
61+
]
6362
}
6463
}
6564

6665
/**
67-
* A block mode being used to form a CryptoSwift `Blowfish` cipher.
66+
* A sink defined in a CSV model.
6867
*/
69-
private class Blowfish extends EcbEncryptionSink {
70-
Blowfish() {
71-
// `blockMode` arg in `Blowfish.init` is a sink
72-
exists(CallExpr call |
73-
call.getStaticTarget()
74-
.(MethodDecl)
75-
.hasQualifiedName("Blowfish", "init(key:blockMode:padding:)") and
76-
call.getArgument(1).getExpr() = this.asExpr()
77-
)
78-
}
68+
private class DefaultEcbEncryptionSink extends EcbEncryptionSink {
69+
DefaultEcbEncryptionSink() { sinkNode(this, "encryption-block-mode") }
7970
}

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow sink for hard-coded encryption key vulnerabilities. That is,
@@ -34,14 +35,11 @@ class HardcodedEncryptionKeyAdditionalTaintStep extends Unit {
3435
private class CryptoSwiftEncryptionKeySink extends HardcodedEncryptionKeySink {
3536
CryptoSwiftEncryptionKeySink() {
3637
// `key` arg in `init` is a sink
37-
exists(CallExpr call, string fName |
38-
call.getStaticTarget()
39-
.(MethodDecl)
40-
.hasQualifiedName([
41-
"AES", "HMAC", "ChaCha20", "CBCMAC", "CMAC", "Poly1305", "Blowfish", "Rabbit"
42-
], fName) and
43-
fName.matches("init(key:%") and
44-
call.getArgument(0).getExpr() = this.asExpr()
38+
exists(NominalTypeDecl c, ConstructorDecl f, CallExpr call |
39+
c.getName() = ["AES", "HMAC", "ChaCha20", "CBCMAC", "CMAC", "Poly1305", "Blowfish", "Rabbit"] and
40+
c.getAMember() = f and
41+
call.getStaticTarget() = f and
42+
call.getArgumentWithLabel("key").getExpr() = this.asExpr()
4543
)
4644
}
4745
}
@@ -51,11 +49,22 @@ private class CryptoSwiftEncryptionKeySink extends HardcodedEncryptionKeySink {
5149
*/
5250
private class RnCryptorEncryptionKeySink extends HardcodedEncryptionKeySink {
5351
RnCryptorEncryptionKeySink() {
54-
exists(ClassOrStructDecl c, MethodDecl f, CallExpr call |
55-
c.getFullName() = ["RNCryptor", "RNEncryptor", "RNDecryptor"] and
52+
exists(NominalTypeDecl c, MethodDecl f, CallExpr call |
53+
c.getFullName() =
54+
[
55+
"RNCryptor", "RNEncryptor", "RNDecryptor", "RNCryptor.EncryptorV3",
56+
"RNCryptor.DecryptorV3"
57+
] and
5658
c.getAMember() = f and
5759
call.getStaticTarget() = f and
5860
call.getArgumentWithLabel(["encryptionKey", "withEncryptionKey"]).getExpr() = this.asExpr()
5961
)
6062
}
6163
}
64+
65+
/**
66+
* A sink defined in a CSV model.
67+
*/
68+
private class DefaultEncryptionKeySink extends HardcodedEncryptionKeySink {
69+
DefaultEncryptionKeySink() { sinkNode(this, "encryption-key") }
70+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow source for insecure TLS configuration vulnerabilities. That is,
@@ -59,3 +60,10 @@ private class NsUrlTlsExtensionsSink extends InsecureTlsExtensionsSink {
5960
)
6061
}
6162
}
63+
64+
/**
65+
* A sink defined in a CSV model.
66+
*/
67+
private class DefaultTlsExtensionsSink extends InsecureTlsExtensionsSink {
68+
DefaultTlsExtensionsSink() { sinkNode(this, "tls-protocol-version") }
69+
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow sink for insufficient hash interation vulnerabilities. That is,
@@ -35,11 +36,18 @@ class InsufficientHashIterationsAdditionalTaintStep extends Unit {
3536
private class CryptoSwiftHashIterationsSink extends InsufficientHashIterationsSink {
3637
CryptoSwiftHashIterationsSink() {
3738
// `iterations` arg in `init` is a sink
38-
exists(ClassOrStructDecl c, ConstructorDecl f, CallExpr call |
39+
exists(NominalTypeDecl c, ConstructorDecl f, CallExpr call |
3940
c.getName() = ["PBKDF1", "PBKDF2"] and
4041
c.getAMember() = f and
4142
call.getStaticTarget() = f and
4243
call.getArgumentWithLabel("iterations").getExpr() = this.asExpr()
4344
)
4445
}
4546
}
47+
48+
/**
49+
* A sink defined in a CSV model.
50+
*/
51+
private class DefaultHashIterationsSink extends InsufficientHashIterationsSink {
52+
DefaultHashIterationsSink() { sinkNode(this, "hash-iteration-count") }
53+
}

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import swift
77
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
89

910
/**
1011
* A dataflow sink for static initialization vector vulnerabilities. That is,
@@ -50,11 +51,22 @@ private class CryptoSwiftInitializationVectorSink extends StaticInitializationVe
5051
*/
5152
private class RnCryptorInitializationVectorSink extends StaticInitializationVectorSink {
5253
RnCryptorInitializationVectorSink() {
53-
exists(ClassOrStructDecl c, MethodDecl f, CallExpr call |
54-
c.getFullName() = ["RNCryptor", "RNEncryptor", "RNDecryptor"] and
54+
exists(NominalTypeDecl c, MethodDecl f, CallExpr call |
55+
c.getFullName() =
56+
[
57+
"RNCryptor", "RNEncryptor", "RNDecryptor", "RNCryptor.EncryptorV3",
58+
"RNCryptor.DecryptorV3"
59+
] and
5560
c.getAMember() = f and
5661
call.getStaticTarget() = f and
5762
call.getArgumentWithLabel(["iv", "IV"]).getExpr() = this.asExpr()
5863
)
5964
}
6065
}
66+
67+
/**
68+
* A sink defined in a CSV model.
69+
*/
70+
private class DefaultInitializationVectorSink extends StaticInitializationVectorSink {
71+
DefaultInitializationVectorSink() { sinkNode(this, "encryption-iv") }
72+
}

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

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import swift
77
import codeql.swift.security.SensitiveExprs
88
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.ExternalFlow
910

1011
/**
11-
* A dataflow sink for weak sensitive data hashing vulnerabilities.
12+
* A dataflow sink for weak sensitive data hashing vulnerabilities. That is,
13+
* a `DataFlow::Node` that is passed into a weak hashing function.
1214
*/
1315
abstract class WeakSensitiveDataHashingSink extends DataFlow::Node {
1416
/**
@@ -33,21 +35,28 @@ class WeakSensitiveDataHashingAdditionalTaintStep extends Unit {
3335
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
3436
}
3537

38+
private class WeakHashingSinks extends SinkModelCsv {
39+
override predicate row(string row) {
40+
row =
41+
[
42+
// CryptoKit
43+
";Insecure.MD5;true;hash(data:);;;Argument[0];weak-hash-input-MD5",
44+
";Insecure.MD5;true;update(data:);;;Argument[0];weak-hash-input-MD5",
45+
";Insecure.MD5;true;update(bufferPointer:);;;Argument[0];weak-hash-input-MD5",
46+
";Insecure.SHA1;true;hash(data:);;;Argument[0];weak-hash-input-SHA1",
47+
";Insecure.SHA1;true;update(data:);;;Argument[0];weak-hash-input-SHA1",
48+
";Insecure.SHA1;true;update(bufferPointer:);;;Argument[0];weak-hash-input-SHA1",
49+
]
50+
}
51+
}
52+
3653
/**
37-
* A sink for the CryptoSwift library.
54+
* A sink defined in a CSV model.
3855
*/
39-
private class CryptoSwiftWeakHashingSink extends WeakSensitiveDataHashingSink {
56+
private class DefaultWeakHashingSink extends WeakSensitiveDataHashingSink {
4057
string algorithm;
4158

42-
CryptoSwiftWeakHashingSink() {
43-
exists(ApplyExpr call, FuncDecl func |
44-
call.getAnArgument().getExpr() = this.asExpr() and
45-
call.getStaticTarget() = func and
46-
func.getName().matches(["hash(%", "update(%"]) and
47-
algorithm = func.getEnclosingDecl().(ClassOrStructDecl).getName() and
48-
algorithm = ["MD5", "SHA1"]
49-
)
50-
}
59+
DefaultWeakHashingSink() { sinkNode(this, "weak-hash-input-" + algorithm) }
5160

5261
override string getAlgorithm() { result = algorithm }
5362
}

0 commit comments

Comments
 (0)