Skip to content

Commit 7b62bed

Browse files
geoffw0geoffw0
authored
Merge pull request github#10947 from karimhamdanali/swift-pbe-iterations
Swift: detect hash functions with low # of iterations
2 parents 1cd3084 + 3911f3b commit 7b62bed

File tree

6 files changed

+171
-0
lines changed

6 files changed

+171
-0
lines changed

‎swift/ql/src/queries/Security/CWE-916/InsufficientHashIterations.qhelp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Storing cryptographic hashes of passwords is standard security practice, but it is equally important to select the right hashing scheme. If an attacker obtains the hashed passwords of an application, the password hashing scheme should still prevent the attacker from easily obtaining the original cleartext passwords.</p>
7+
<p>A good password hashing scheme requires a computation that cannot be done efficiently. Hashing schemes with low number of iterations are efficiently computable, and are therefore not suitable for password hashing.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use the OWASP recommendation for sufficient number of iterations (currently, that is greater than or equal to 120,000) for password hashing schemes.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>The following example shows a few cases where a password hashing scheme is instantiated. In the 'BAD' cases, the scheme is initialized with insufficient iterations, making it susceptible to password cracking attacks. In the 'GOOD' cases, the scheme is initialized with at least 120,000 iterations, which protects the hashed data against recovery.</p>
16+
<sample src="InsufficientHashIterations.swift" />
17+
</example>
18+
19+
<references>
20+
<li>Password-Based Cryptography Specification Version 2.0. 2000.<a href="https://www.rfc-editor.org/rfc/rfc2898">RFC2898</a>.</li>
21+
<li>OWASP <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet.</a></li>
22+
</references>
23+
</qhelp>

‎swift/ql/src/queries/Security/CWE-916/InsufficientHashIterations.ql

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/**
2+
* @name Insufficient hash iterations
3+
* @description Using hash functions with fewer than 120,000 iterations is insufficient to protect passwords because a cracking attack will require a low level of computational effort.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.8
7+
* @precision high
8+
* @id swift/insufficient-hash-iterations
9+
* @tags security
10+
* external/cwe/cwe-916
11+
*/
12+
13+
import swift
14+
import codeql.swift.dataflow.DataFlow
15+
import codeql.swift.dataflow.TaintTracking
16+
import DataFlow::PathGraph
17+
18+
/**
19+
* An `Expr` that is used to initialize a password-based encryption key.
20+
*/
21+
abstract class IterationsSource extends Expr { }
22+
23+
/**
24+
* A literal integer that is 120,000 or less is a source of taint for iterations.
25+
*/
26+
class IntLiteralSource extends IterationsSource instanceof IntegerLiteralExpr {
27+
IntLiteralSource() { this.getStringValue().toInt() < 120000 }
28+
}
29+
30+
/**
31+
* A class for all ways to set the iterations of hash function.
32+
*/
33+
class InsufficientHashIterationsSink extends Expr {
34+
InsufficientHashIterationsSink() {
35+
// `iterations` arg in `init` is a sink
36+
exists(ClassOrStructDecl c, AbstractFunctionDecl f, CallExpr call, int arg |
37+
c.getFullName() = ["PBKDF1", "PBKDF2"] and
38+
c.getAMember() = f and
39+
f.getName().matches("init(%iterations:%") and
40+
call.getStaticTarget() = f and
41+
f.getParam(pragma[only_bind_into](arg)).getName() = "iterations" and
42+
call.getArgument(pragma[only_bind_into](arg)).getExpr() = this
43+
)
44+
}
45+
}
46+
47+
/**
48+
* A dataflow configuration from the hash iterations source to expressions that use
49+
* it to initialize hash functions.
50+
*/
51+
class InsufficientHashIterationsConfig extends TaintTracking::Configuration {
52+
InsufficientHashIterationsConfig() { this = "InsufficientHashIterationsConfig" }
53+
54+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof IterationsSource }
55+
56+
override predicate isSink(DataFlow::Node node) {
57+
node.asExpr() instanceof InsufficientHashIterationsSink
58+
}
59+
}
60+
61+
// The query itself
62+
from
63+
InsufficientHashIterationsConfig config, DataFlow::PathNode sourceNode,
64+
DataFlow::PathNode sinkNode
65+
where config.hasFlowPath(sourceNode, sinkNode)
66+
select sinkNode.getNode(), sourceNode, sinkNode,
67+
"The value '" + sourceNode.getNode().toString() +
68+
"' is an insufficient number of iterations for secure password hashing."

‎swift/ql/src/queries/Security/CWE-916/InsufficientHashIterations.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
func hash() {
3+
// ...
4+
5+
// BAD: Using insufficient (that is, < 120,000) iterations for password hashing
6+
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 90000, keyLength: 0)
7+
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 90000, keyLength: 0)
8+
9+
// GOOD: Using sufficient (that is, >= 120,000) iterations for password hashing
10+
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 120120, keyLength: 0)
11+
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 310000, keyLength: 0)
12+
13+
// ...
14+
}

‎swift/ql/test/query-tests/Security/CWE-916/InsufficientHashIterations.expected

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
edges
2+
| test.swift:20:45:20:45 | 99999 : | test.swift:33:22:33:43 | call to getLowIterationCount() : |
3+
| test.swift:33:22:33:43 | call to getLowIterationCount() : | test.swift:37:84:37:84 | lowIterations |
4+
| test.swift:33:22:33:43 | call to getLowIterationCount() : | test.swift:44:84:44:84 | lowIterations |
5+
nodes
6+
| test.swift:20:45:20:45 | 99999 : | semmle.label | 99999 : |
7+
| test.swift:33:22:33:43 | call to getLowIterationCount() : | semmle.label | call to getLowIterationCount() : |
8+
| test.swift:37:84:37:84 | lowIterations | semmle.label | lowIterations |
9+
| test.swift:38:84:38:84 | 80000 | semmle.label | 80000 |
10+
| test.swift:44:84:44:84 | lowIterations | semmle.label | lowIterations |
11+
| test.swift:45:84:45:84 | 80000 | semmle.label | 80000 |
12+
subpaths
13+
#select
14+
| test.swift:37:84:37:84 | lowIterations | test.swift:20:45:20:45 | 99999 : | test.swift:37:84:37:84 | lowIterations | The value '99999' is an insufficient number of iterations for secure password hashing. |
15+
| test.swift:38:84:38:84 | 80000 | test.swift:38:84:38:84 | 80000 | test.swift:38:84:38:84 | 80000 | The value '80000' is an insufficient number of iterations for secure password hashing. |
16+
| test.swift:44:84:44:84 | lowIterations | test.swift:20:45:20:45 | 99999 : | test.swift:44:84:44:84 | lowIterations | The value '99999' is an insufficient number of iterations for secure password hashing. |
17+
| test.swift:45:84:45:84 | 80000 | test.swift:45:84:45:84 | 80000 | test.swift:45:84:45:84 | 80000 | The value '80000' is an insufficient number of iterations for secure password hashing. |

‎swift/ql/test/query-tests/Security/CWE-916/InsufficientHashIterations.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/Security/CWE-916/InsufficientHashIterations.ql

‎swift/ql/test/query-tests/Security/CWE-916/test.swift

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
2+
// --- stubs ---
3+
4+
// These stubs roughly follows the same structure as classes from CryptoSwift
5+
enum PKCS5 { }
6+
7+
enum Variant { case md5, sha1, sha2, sha3 }
8+
9+
extension PKCS5 {
10+
struct PBKDF1 {
11+
init(password: Array<UInt8>, salt: Array<UInt8>, variant: Variant = .sha1, iterations: Int = 4096, keyLength: Int? = nil) { }
12+
}
13+
14+
struct PBKDF2 {
15+
init(password: Array<UInt8>, salt: Array<UInt8>, iterations: Int = 4096, keyLength: Int? = nil, variant: Variant = .sha2) { }
16+
}
17+
}
18+
19+
// Helper functions
20+
func getLowIterationCount() -> Int { return 99999 }
21+
22+
func getEnoughIterationCount() -> Int { return 120120 }
23+
24+
func getRandomArray() -> Array<UInt8> {
25+
(0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
26+
}
27+
28+
// --- tests ---
29+
30+
func test() {
31+
let randomArray = getRandomArray()
32+
let variant = Variant.sha2
33+
let lowIterations = getLowIterationCount()
34+
let enoughIterations = getEnoughIterationCount()
35+
36+
// PBKDF1 test cases
37+
let pbkdf1b1 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: lowIterations, keyLength: 0) // BAD
38+
let pbkdf1b2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 80000, keyLength: 0) // BAD
39+
let pbkdf1g1 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: enoughIterations, keyLength: 0) // GOOD
40+
let pbkdf1g2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 120120, keyLength: 0) // GOOD
41+
42+
43+
// PBKDF2 test cases
44+
let pbkdf2b1 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: lowIterations, keyLength: 0) // BAD
45+
let pbkdf2b2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 80000, keyLength: 0) // BAD
46+
let pbkdf2g1 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: enoughIterations, keyLength: 0) // GOOD
47+
let pbkdf2g2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 120120, keyLength: 0) // GOOD
48+
}

0 commit comments

Comments
 (0)