Skip to content

Commit c4b2519

Browse files
karimhamdanalikarimhamdanali
committed
initial draft of the Swift query for CWE-916
1 parent 7e2c06d commit c4b2519

File tree

6 files changed

+156
-0
lines changed

6 files changed

+156
-0
lines changed

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

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Using hash functions with less than 1,000 iterations is not secure. That scheme is vulnerable to password cracking attacks due to having an insufficient level of computational effort.</p>
7+
</overview>
8+
9+
<recommendation>
10+
<p>Use sufficient number of iterations (i.e., greater than or equal 1000) for generating password-based keys.</p>
11+
</recommendation>
12+
13+
<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>
15+
<sample src="InsufficientHashIterations.swift" />
16+
</example>
17+
</qhelp>

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

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* @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.
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 1000 or less is a source of taint for iterations.
25+
*/
26+
class IntLiteralSource extends IterationsSource instanceof IntegerLiteralExpr {
27+
IntLiteralSource() { this.getStringValue().toInt() >= 1000 }
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 |
37+
c.getFullName() = "PKCS5.PBKDF1" and
38+
c.getAMember() = f and
39+
f.getName().matches("init(%iterations:%") and
40+
call.getStaticTarget() = f and
41+
call.getArgument(2).getExpr() = this
42+
)
43+
or
44+
exists(ClassOrStructDecl c, AbstractFunctionDecl f, CallExpr call |
45+
c.getFullName() = "PKCS5.PBKDF2" and
46+
c.getAMember() = f and
47+
f.getName().matches("init(%iterations:%") and
48+
call.getStaticTarget() = f and
49+
call.getArgument(3).getExpr() = this
50+
)
51+
}
52+
}
53+
54+
/**
55+
* A dataflow configuration from the hash iterations source to expressions that use
56+
* it to initialize hash functions.
57+
*/
58+
class InsufficientHashIterationsConfig extends TaintTracking::Configuration {
59+
InsufficientHashIterationsConfig() { this = "InsufficientHashIterationsConfig" }
60+
61+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof IterationsSource }
62+
63+
override predicate isSink(DataFlow::Node node) {
64+
node.asExpr() instanceof InsufficientHashIterationsSink
65+
}
66+
}
67+
68+
// The query itself
69+
from
70+
InsufficientHashIterationsConfig config, DataFlow::PathNode sourceNode,
71+
DataFlow::PathNode sinkNode
72+
where config.hasFlowPath(sourceNode, sinkNode)
73+
select sinkNode.getNode(), sourceNode, sinkNode,
74+
"The hash function '" + sinkNode.getNode().toString() +
75+
"' has been initialized with an insufficient number of iterations from $@.", sourceNode,
76+
sourceNode.getNode().toString()

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

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
func encrypt() {
3+
// ...
4+
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)
8+
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)
12+
13+
// ...
14+
}

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

Whitespace-only changes.

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

+1
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

+48
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 999 }
21+
22+
func getEnoughIterationCount() -> Int { return 1000 }
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: 800, 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: 1200, keyLength: 0) // GOOD
41+
42+
43+
// PBKDF2 test cases
44+
let pbkdf2b1 = try PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: lowIterations, keyLength: 0) // BAD
45+
let pbkdf2b2 = try PKCS5.PBKDF2(password: randomArray, salt: randomArray, iterations: 800, keyLength: 0) // BAD
46+
let pbkdf2g1 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: enoughIterations, keyLength: 0) // GOOD
47+
let pbkdf2g2 = PKCS5.PBKDF1(password: randomArray, salt: randomArray, iterations: 1200, keyLength: 0) // GOOD
48+
}

0 commit comments

Comments
 (0)