Skip to content

Commit 18dd0f6

Browse files
karimhamdanalikarimhamdanali
committed
update iterations threshold to most recent OWASP recommendation
which is at least 120,000 iterations for secure password hashing
1 parent 408c7be commit 18dd0f6

File tree

5 files changed

+25
-25
lines changed

5 files changed

+25
-25
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
</overview>
88

99
<recommendation>
10-
<p>Use sufficient number of iterations (that is, greater than or equal 1000) for generating password-based keys.</p>
10+
<p>Use sufficient number of iterations (that is, greater than or equal 120000) for generating password-based keys.</p>
1111
</recommendation>
1212

1313
<example>
14-
<p>The following example shows a few cases of instantiating a password-based key. In the 'BAD' cases, the key is initialized with insufficient iterations, making it susceptible to password cracking attacks. In the 'GOOD' cases, the key is initialized with at least 1000 iterations, which protects the encrypted data against recovery.</p>
14+
<p>The following example shows a few cases of instantiating a password-based key. In the 'BAD' cases, the key is initialized with insufficient iterations, making it susceptible to password cracking attacks. In the 'GOOD' cases, the key is initialized with at least 120000 iterations, which protects the encrypted data against recovery.</p>
1515
<sample src="InsufficientHashIterations.swift" />
1616
</example>
1717

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name Insufficient hash iterations
3-
* @description Using hash functions with < 1000 iterations is not secure, because that scheme leads to password cracking attacks due to having an insufficient level of computational effort.
3+
* @description Using hash functions with less than 120,000 iterations is not secure, because that scheme leads to password cracking attacks due to having an insufficient level of computational effort.
44
* @kind path-problem
55
* @problem.severity error
66
* @security-severity 7.8
@@ -21,10 +21,10 @@ import DataFlow::PathGraph
2121
abstract class IterationsSource extends Expr { }
2222

2323
/**
24-
* A literal integer that is 1000 or less is a source of taint for iterations.
24+
* A literal integer that is 120,000 or less is a source of taint for iterations.
2525
*/
2626
class IntLiteralSource extends IterationsSource instanceof IntegerLiteralExpr {
27-
IntLiteralSource() { this.getStringValue().toInt() < 1000 }
27+
IntLiteralSource() { this.getStringValue().toInt() < 120000 }
2828
}
2929

3030
/**

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
func encrypt() {
33
// ...
44

5-
// BAD: Using insufficient (i.e., < 1000) hash iterations keys for encryption
6-
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 900, keyLength: 0)
7-
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 900, keyLength: 0)
5+
// BAD: Using insufficient (i.e., < 120,000) hash iterations keys for encryption
6+
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 90000, keyLength: 0)
7+
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 90000, keyLength: 0)
88

9-
// GOOD: Using sufficient (i.e., >= 1000) hash iterations keys for encryption
10-
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 1100, keyLength: 0)
11-
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 1100, keyLength: 0)
9+
// GOOD: Using sufficient (i.e., >= 120,000) hash iterations keys for encryption
10+
_ = try PKCS5.PBKDF1(password: getRandomArray(), salt: getRandomArray(), iterations: 120120, keyLength: 0)
11+
_ = try PKCS5.PBKDF2(password: getRandomArray(), salt: getRandomArray(), iterations: 120120, keyLength: 0)
1212

1313
// ...
1414
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
edges
2-
| test.swift:20:45:20:45 | 999 : | test.swift:33:22:33:43 | call to getLowIterationCount() : |
2+
| test.swift:20:45:20:45 | 99999 : | test.swift:33:22:33:43 | call to getLowIterationCount() : |
33
| test.swift:33:22:33:43 | call to getLowIterationCount() : | test.swift:37:84:37:84 | lowIterations |
44
| test.swift:33:22:33:43 | call to getLowIterationCount() : | test.swift:44:84:44:84 | lowIterations |
55
nodes
6-
| test.swift:20:45:20:45 | 999 : | semmle.label | 999 : |
6+
| test.swift:20:45:20:45 | 99999 : | semmle.label | 99999 : |
77
| test.swift:33:22:33:43 | call to getLowIterationCount() : | semmle.label | call to getLowIterationCount() : |
88
| test.swift:37:84:37:84 | lowIterations | semmle.label | lowIterations |
9-
| test.swift:38:84:38:84 | 800 | semmle.label | 800 |
9+
| test.swift:38:84:38:84 | 80000 | semmle.label | 80000 |
1010
| test.swift:44:84:44:84 | lowIterations | semmle.label | lowIterations |
11-
| test.swift:45:84:45:84 | 800 | semmle.label | 800 |
11+
| test.swift:45:84:45:84 | 80000 | semmle.label | 80000 |
1212
subpaths
1313
#select
14-
| test.swift:37:84:37:84 | lowIterations | test.swift:20:45:20:45 | 999 : | test.swift:37:84:37:84 | lowIterations | The value '999' is an insufficient number of iterations for secure password hashing. |
15-
| test.swift:38:84:38:84 | 800 | test.swift:38:84:38:84 | 800 | test.swift:38:84:38:84 | 800 | The value '800' is an insufficient number of iterations for secure password hashing. |
16-
| test.swift:44:84:44:84 | lowIterations | test.swift:20:45:20:45 | 999 : | test.swift:44:84:44:84 | lowIterations | The value '999' is an insufficient number of iterations for secure password hashing. |
17-
| test.swift:45:84:45:84 | 800 | test.swift:45:84:45:84 | 800 | test.swift:45:84:45:84 | 800 | The value '800' is an insufficient number of iterations for secure password hashing. |
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/test.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ extension PKCS5 {
1717
}
1818

1919
// Helper functions
20-
func getLowIterationCount() -> Int { return 999 }
20+
func getLowIterationCount() -> Int { return 99999 }
2121

22-
func getEnoughIterationCount() -> Int { return 1000 }
22+
func getEnoughIterationCount() -> Int { return 120120 }
2323

2424
func getRandomArray() -> Array<UInt8> {
2525
(0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
@@ -35,14 +35,14 @@ func test() {
3535

3636
// PBKDF1 test cases
3737
let pbkdf1b1 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: lowIterations, keyLength: 0) // BAD
38-
let pbkdf1b2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 800, keyLength: 0) // BAD
38+
let pbkdf1b2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 80000, keyLength: 0) // BAD
3939
let pbkdf1g1 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: enoughIterations, keyLength: 0) // GOOD
40-
let pbkdf1g2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 1200, keyLength: 0) // GOOD
40+
let pbkdf1g2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 120120, keyLength: 0) // GOOD
4141

4242

4343
// PBKDF2 test cases
4444
let pbkdf2b1 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: lowIterations, keyLength: 0) // BAD
45-
let pbkdf2b2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 800, keyLength: 0) // BAD
45+
let pbkdf2b2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 80000, keyLength: 0) // BAD
4646
let pbkdf2g1 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: enoughIterations, keyLength: 0) // GOOD
47-
let pbkdf2g2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 1200, keyLength: 0) // GOOD
47+
let pbkdf2g2 = PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 120120, keyLength: 0) // GOOD
4848
}

0 commit comments

Comments
 (0)