Skip to content

Commit 6636c76

Browse files
geoffw0geoffw0
authored
Merge pull request #15122 from geoffw0/pwhash
Swift: Query for Use of an inappropriate cryptographic hashing algorithm on passwords
2 parents f4df5c9 + 0aec2b1 commit 6636c76

22 files changed

+786
-202
lines changed

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Security.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ private import codeql.swift.dataflow.ExternalFlow
88

99
private class SensitiveSources extends SourceModelCsv {
1010
override predicate row(string row) {
11-
row = ";;false;SecKeyCopyExternalRepresentation(_:_:);;;ReturnValue;sensitive-credential"
11+
row = ";;false;SecKeyCopyExternalRepresentation(_:_:);;;ReturnValue;sensitive-password"
1212
}
1313
}

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ private import codeql.swift.dataflow.DataFlow
1010
private import codeql.swift.dataflow.ExternalFlow
1111

1212
private newtype TSensitiveDataType =
13+
TPassword() or
1314
TCredential() or
1415
TPrivateInfo()
1516

@@ -26,18 +27,32 @@ abstract class SensitiveDataType extends TSensitiveDataType {
2627
}
2728

2829
/**
29-
* The type of sensitive expression for passwords and other credentials.
30+
* The type of sensitive expression for passwords.
31+
*/
32+
class SensitivePassword extends SensitiveDataType, TPassword {
33+
override string toString() { result = "password" }
34+
35+
override string getRegexp() {
36+
result = HeuristicNames::maybeSensitiveRegexp(SensitiveDataClassification::password())
37+
or
38+
result = "(?is).*pass.?phrase.*"
39+
}
40+
}
41+
42+
/**
43+
* The type of sensitive expression for credentials and secrets other than passwords.
3044
*/
3145
class SensitiveCredential extends SensitiveDataType, TCredential {
3246
override string toString() { result = "credential" }
3347

3448
override string getRegexp() {
3549
exists(SensitiveDataClassification classification |
50+
not classification = SensitiveDataClassification::password() and // covered by `SensitivePassword`
3651
not classification = SensitiveDataClassification::id() and // not accurate enough
3752
result = HeuristicNames::maybeSensitiveRegexp(classification)
3853
)
3954
or
40-
result = "(?is).*((account|accnt|licen(se|ce)).?(id|key)|one.?time.?code|pass.?phrase).*"
55+
result = "(?is).*((account|accnt|licen(se|ce)).?(id|key)|one.?time.?code).*"
4156
}
4257
}
4358

@@ -57,7 +72,8 @@ class SensitivePrivateInfo extends SensitiveDataType, TPrivateInfo {
5772
// Contact information, such as home addresses
5873
"post.?code|zip.?code|home.?addr|" +
5974
// and telephone numbers
60-
"(mob(ile)?|home).?(num|no|tel|phone)|(tel|fax).?(num|no|phone)|" + "emergency.?contact|" +
75+
"(mob(ile)?|home).?(num|no|tel|phone)|(tel|fax|phone).?(num|no)|telephone|" +
76+
"emergency.?contact|" +
6177
// Geographic location - where the user is (or was)
6278
"l(atitude|ongitude)|nationality|" +
6379
// Financial data - such as credit card numbers, salary, bank accounts, and debts
@@ -176,6 +192,11 @@ class SensitiveExpr extends Expr {
176192
not label.regexpMatch(regexpProbablySafe())
177193
or
178194
(
195+
// modeled sensitive password
196+
sourceNode(DataFlow::exprNode(this), "sensitive-password") and
197+
sensitiveType = TPassword() and
198+
label = "password"
199+
or
179200
// modeled sensitive credential
180201
sourceNode(DataFlow::exprNode(this), "sensitive-credential") and
181202
sensitiveType = TCredential() and

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* Provides classes and predicates for reasoning about use of inappropriate
3+
* cryptographic hashing algorithms on passwords.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
9+
private import codeql.swift.security.WeakSensitiveDataHashingExtensions
10+
11+
/**
12+
* A dataflow sink for weak password hashing vulnerabilities. That is,
13+
* a `DataFlow::Node` that is passed into a weak password hashing function.
14+
*/
15+
abstract class WeakPasswordHashingSink extends DataFlow::Node {
16+
/**
17+
* Gets the name of the hashing algorithm, for display.
18+
*/
19+
abstract string getAlgorithm();
20+
}
21+
22+
/**
23+
* A barrier for weak password hashing vulnerabilities.
24+
*/
25+
abstract class WeakPasswordHashingBarrier extends DataFlow::Node { }
26+
27+
/**
28+
* A unit class for adding additional flow steps.
29+
*/
30+
class WeakPasswordHashingAdditionalFlowStep extends Unit {
31+
/**
32+
* Holds if the step from `node1` to `node2` should be considered a flow
33+
* step for paths related to weak password hashing vulnerabilities.
34+
*/
35+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
36+
}
37+
38+
/**
39+
* A sink inherited from weak sensitive data hashing. Password hashing has
40+
* stronger requirements than sensitive data hashing, since (in addition to
41+
* its particular qualities) a password *is* sensitive data. Thus, any sink
42+
* for the weak sensitive data hashing query is a sink for weak password
43+
* hashing as well.
44+
*/
45+
private class InheritedWeakPasswordHashingSink extends WeakPasswordHashingSink instanceof WeakSensitiveDataHashingSink
46+
{
47+
override string getAlgorithm() { result = this.(WeakSensitiveDataHashingSink).getAlgorithm() }
48+
}
49+
50+
private class WeakSensitiveDataHashingSinks extends SinkModelCsv {
51+
override predicate row(string row) {
52+
row =
53+
[
54+
// CryptoKit
55+
// (SHA-256, SHA-384 and SHA-512 are all variants of the SHA-2 algorithm)
56+
";SHA256;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA256",
57+
";SHA256;true;update(data:);;;Argument[0];weak-password-hash-input-SHA256",
58+
";SHA256;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA256",
59+
";SHA384;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA384",
60+
";SHA384;true;update(data:);;;Argument[0];weak-password-hash-input-SHA384",
61+
";SHA384;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA384",
62+
";SHA512;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA512",
63+
";SHA512;true;update(data:);;;Argument[0];weak-password-hash-input-SHA512",
64+
";SHA512;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA512",
65+
// CryptoSwift
66+
";SHA2;true;calculate(for:);;;Argument[0];weak-password-hash-input-SHA2",
67+
";SHA2;true;callAsFunction(_:);;;Argument[0];weak-password-hash-input-SHA2",
68+
";SHA2;true;process64(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
69+
";SHA2;true;process32(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
70+
";SHA2;true;update(withBytes:isLast:);;;Argument[0];weak-password-hash-input-SHA2",
71+
";SHA3;true;calculate(for:);;;Argument[0];weak-password-hash-input-SHA2",
72+
";SHA3;true;callAsFunction(_:);;;Argument[0];weak-password-hash-input-SHA2",
73+
";SHA3;true;process(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
74+
";SHA3;true;update(withBytes:isLast:);;;Argument[0];weak-password-hash-input-SHA2",
75+
";Digest;true;sha2(_:variant:);;;Argument[0];weak-password-hash-input-SHA2",
76+
";Digest;true;sha3(_:variant:);;;Argument[0];weak-password-hash-input-SHA3",
77+
";Digest;true;sha224(_:);;;Argument[0];weak-password-hash-input-SHA224",
78+
";Digest;true;sha256(_:);;;Argument[0];weak-password-hash-input-SHA256",
79+
";Digest;true;sha384(_:);;;Argument[0];weak-password-hash-input-SHA384",
80+
";Digest;true;sha512(_:);;;Argument[0];weak-password-hash-input-SHA512",
81+
";Array;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
82+
";Array;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
83+
";Array;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
84+
";Array;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
85+
";Array;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
86+
";Array;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
87+
";Data;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
88+
";Data;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
89+
";Data;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
90+
";Data;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
91+
";Data;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
92+
";Data;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
93+
";String;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
94+
";String;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
95+
";String;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
96+
";String;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
97+
";String;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
98+
";String;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
99+
]
100+
}
101+
}
102+
103+
/**
104+
* A sink defined in a CSV model.
105+
*/
106+
private class DefaultWeakPasswordHashingSink extends WeakPasswordHashingSink {
107+
string algorithm;
108+
109+
DefaultWeakPasswordHashingSink() { sinkNode(this, "weak-password-hash-input-" + algorithm) }
110+
111+
override string getAlgorithm() { result = algorithm }
112+
}
113+
114+
/**
115+
* A barrier for weak password hashing, when it occurs inside of
116+
* certain cryptographic algorithms as part of their design.
117+
*/
118+
class WeakPasswordHashingImplementationBarrier extends WeakPasswordHashingBarrier {
119+
WeakPasswordHashingImplementationBarrier() {
120+
this.asParameter()
121+
.getDeclaringFunction()
122+
.(Function)
123+
.getDeclaringDecl*()
124+
.(NominalTypeDecl)
125+
.getName() = ["HMAC", "PBKDF1", "PBKDF2"]
126+
}
127+
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Provides a taint tracking configuration to find use of inappropriate
3+
* cryptographic hashing algorithms on passwords.
4+
*/
5+
6+
import swift
7+
import codeql.swift.security.SensitiveExprs
8+
import codeql.swift.dataflow.TaintTracking
9+
import codeql.swift.security.WeakPasswordHashingExtensions
10+
11+
/**
12+
* A taint tracking configuration from password expressions to inappropriate
13+
* hashing sinks.
14+
*/
15+
module WeakPasswordHashingConfig implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node node) {
17+
exists(SensitiveExpr se |
18+
node.asExpr() = se and
19+
se.getSensitiveType() instanceof SensitivePassword
20+
)
21+
}
22+
23+
predicate isSink(DataFlow::Node node) { node instanceof WeakPasswordHashingSink }
24+
25+
predicate isBarrier(DataFlow::Node node) { node instanceof WeakPasswordHashingBarrier }
26+
27+
predicate isBarrierIn(DataFlow::Node node) {
28+
// make sources barriers so that we only report the closest instance
29+
isSource(node)
30+
}
31+
32+
predicate isBarrierOut(DataFlow::Node node) {
33+
// make sinks barriers so that we only report the closest instance
34+
isSink(node)
35+
}
36+
37+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
38+
any(WeakPasswordHashingAdditionalFlowStep s).step(nodeFrom, nodeTo)
39+
}
40+
}
41+
42+
module WeakPasswordHashingFlow = TaintTracking::Global<WeakPasswordHashingConfig>;

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55

66
import swift
7-
import codeql.swift.security.SensitiveExprs
87
import codeql.swift.dataflow.DataFlow
98
import codeql.swift.dataflow.ExternalFlow
109

@@ -35,7 +34,7 @@ class WeakSensitiveDataHashingAdditionalFlowStep extends Unit {
3534
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
3635
}
3736

38-
private class WeakHashingSinks extends SinkModelCsv {
37+
private class WeakSensitiveDataHashingSinks extends SinkModelCsv {
3938
override predicate row(string row) {
4039
row =
4140
[
@@ -49,9 +48,11 @@ private class WeakHashingSinks extends SinkModelCsv {
4948
// CryptoSwift
5049
";MD5;true;calculate(for:);;;Argument[0];weak-hash-input-MD5",
5150
";MD5;true;callAsFunction(_:);;;Argument[0];weak-hash-input-MD5",
51+
";MD5;true;process(block:currentHash:);;;Argument[0];weak-hash-input-MD5",
5252
";MD5;true;update(withBytes:isLast:);;;Argument[0];weak-hash-input-MD5",
5353
";SHA1;true;calculate(for:);;;Argument[0];weak-hash-input-SHA1",
5454
";SHA1;true;callAsFunction(_:);;;Argument[0];weak-hash-input-SHA1",
55+
";SHA1;true;process(block:currentHash:);;;Argument[0];weak-hash-input-SHA1",
5556
";SHA1;true;update(withBytes:isLast:);;;Argument[0];weak-hash-input-SHA1",
5657
";Digest;true;md5(_:);;;Argument[0];weak-hash-input-MD5",
5758
";Digest;true;sha1(_:);;;Argument[0];weak-hash-input-SHA1",
@@ -68,10 +69,10 @@ private class WeakHashingSinks extends SinkModelCsv {
6869
/**
6970
* A sink defined in a CSV model.
7071
*/
71-
private class DefaultWeakHashingSink extends WeakSensitiveDataHashingSink {
72+
private class DefaultWeakSenitiveDataHashingSink extends WeakSensitiveDataHashingSink {
7273
string algorithm;
7374

74-
DefaultWeakHashingSink() { sinkNode(this, "weak-hash-input-" + algorithm) }
75+
DefaultWeakSenitiveDataHashingSink() { sinkNode(this, "weak-hash-input-" + algorithm) }
7576

7677
override string getAlgorithm() { result = algorithm }
7778
}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@ import codeql.swift.security.WeakSensitiveDataHashingExtensions
1313
* A taint tracking configuration from sensitive expressions to broken or weak
1414
* hashing sinks.
1515
*/
16-
module WeakHashingConfig implements DataFlow::ConfigSig {
17-
predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
16+
module WeakSensitiveDataHashingConfig implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node node) {
18+
exists(SensitiveExpr se |
19+
node.asExpr() = se and
20+
not se.getSensitiveType() instanceof SensitivePassword // responsibility of the weak password hashing query
21+
)
22+
}
1823

1924
predicate isSink(DataFlow::Node node) { node instanceof WeakSensitiveDataHashingSink }
2025

@@ -35,4 +40,8 @@ module WeakHashingConfig implements DataFlow::ConfigSig {
3540
}
3641
}
3742

38-
module WeakHashingFlow = TaintTracking::Global<WeakHashingConfig>;
43+
deprecated module WeakHashingConfig = WeakSensitiveDataHashingConfig;
44+
45+
module WeakSensitiveDataHashingFlow = TaintTracking::Global<WeakSensitiveDataHashingConfig>;
46+
47+
deprecated module WeakHashingFlow = WeakSensitiveDataHashingFlow;

‎swift/ql/src/change-notes/2023-12-15-weak-password-hashing.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: newQuery
3+
---
4+
5+
* Added new query "Use of an inappropriate cryptographic hashing algorithm on passwords" (`swift/weak-password-hashing`). This query detects use of inappropriate hashing algorithms for password hashing. Some of the results of this query are new, others would previously have been reported by the "Use of a broken or weak cryptographic hashing algorithm on sensitive data" (`swift/weak-sensitive-data-hashing`) query.

0 commit comments

Comments
 (0)