Skip to content

Commit 5766ff2

Browse files
karimhamdanalikarimhamdanali
authored
Merge pull request github#10993 from karimhamdanali/swift-pbe-constant-salts
Swift: detect the use of constant salts
2 parents 577f1a5 + 1756fea commit 5766ff2

File tree

6 files changed

+194
-0
lines changed

6 files changed

+194
-0
lines changed

‎swift/ql/src/queries/Security/CWE-760/ConstantSalt.qhelp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Constant salts should not be used for password hashing. Data hashed using constant salts are vulnerable to dictionary attacks, enabling attackers to recover the original input.</p>
7+
</overview>
8+
9+
<recommendation>
10+
<p>Use randomly generated salts to securely hash input data.</p>
11+
</recommendation>
12+
13+
<example>
14+
<p>The following example shows a few cases of hashing input data. In the 'BAD' cases, the salt is constant, making the generated hashes vulnerable to dictionary attacks. In the 'GOOD' cases, the salt is randomly generated, which protects the hashed data against recovery.</p>
15+
<sample src="ConstantSalt.swift" />
16+
</example>
17+
18+
<references>
19+
<li><a href="https://www.okta.com/blog/2019/03/what-are-salted-passwords-and-password-hashing/">What are Salted Passwords and Password Hashing?</a></li>
20+
</references>
21+
</qhelp>

‎swift/ql/src/queries/Security/CWE-760/ConstantSalt.ql

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @name Use of constant salts
3+
* @description Using constant salts for password hashing is not secure because potential attackers can precompute the hash value via dictionary attacks.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id swift/constant-salt
9+
* @tags security
10+
* external/cwe/cwe-760
11+
*/
12+
13+
import swift
14+
import codeql.swift.dataflow.DataFlow
15+
import codeql.swift.dataflow.TaintTracking
16+
import codeql.swift.dataflow.FlowSteps
17+
import DataFlow::PathGraph
18+
19+
/**
20+
* A constant salt is created through either a byte array or string literals.
21+
*/
22+
class ConstantSaltSource extends Expr {
23+
ConstantSaltSource() {
24+
this = any(ArrayExpr arr | arr.getType().getName() = "Array<UInt8>") or
25+
this instanceof StringLiteralExpr
26+
}
27+
}
28+
29+
/**
30+
* A class for all ways to use a constant salt.
31+
*/
32+
class ConstantSaltSink extends Expr {
33+
ConstantSaltSink() {
34+
// `salt` arg in `init` is a sink
35+
exists(ClassOrStructDecl c, AbstractFunctionDecl f, CallExpr call, int arg |
36+
c.getFullName() = ["HKDF", "PBKDF1", "PBKDF2", "Scrypt"] and
37+
c.getAMember() = f and
38+
f.getName().matches("%init(%salt:%") and
39+
call.getStaticTarget() = f and
40+
f.getParam(pragma[only_bind_into](arg)).getName() = "salt" and
41+
call.getArgument(pragma[only_bind_into](arg)).getExpr() = this
42+
)
43+
}
44+
}
45+
46+
/**
47+
* A taint configuration from the source of constants salts to expressions that use
48+
* them to initialize password-based enecryption keys.
49+
*/
50+
class ConstantSaltConfig extends TaintTracking::Configuration {
51+
ConstantSaltConfig() { this = "ConstantSaltConfig" }
52+
53+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSource }
54+
55+
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSink }
56+
}
57+
58+
// The query itself
59+
from ConstantSaltConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
60+
where config.hasFlowPath(sourceNode, sinkNode)
61+
select sinkNode.getNode(), sourceNode, sinkNode,
62+
"The value '" + sourceNode.getNode().toString() +
63+
"' is used as a constant salt, which is insecure for hashing passwords."

‎swift/ql/src/queries/Security/CWE-760/ConstantSalt.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
2+
func encrypt(padding : Padding) {
3+
// ...
4+
5+
// BAD: Using constant salts for hashing
6+
let salt: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05]
7+
let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
8+
_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
9+
_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
10+
_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
11+
_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
12+
13+
// GOOD: Using randomly generated salts for hashing
14+
let salt = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
15+
let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
16+
_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
17+
_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
18+
_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
19+
_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
20+
21+
// ...
22+
}

‎swift/ql/test/query-tests/Security/CWE-760/ConstantSalt.expected

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
edges
2+
| test.swift:43:35:43:130 | [...] : | test.swift:51:49:51:49 | constantSalt |
3+
| test.swift:43:35:43:130 | [...] : | test.swift:56:59:56:59 | constantSalt |
4+
| test.swift:43:35:43:130 | [...] : | test.swift:62:59:62:59 | constantSalt |
5+
| test.swift:43:35:43:130 | [...] : | test.swift:67:53:67:53 | constantSalt |
6+
nodes
7+
| test.swift:43:35:43:130 | [...] : | semmle.label | [...] : |
8+
| test.swift:51:49:51:49 | constantSalt | semmle.label | constantSalt |
9+
| test.swift:56:59:56:59 | constantSalt | semmle.label | constantSalt |
10+
| test.swift:62:59:62:59 | constantSalt | semmle.label | constantSalt |
11+
| test.swift:67:53:67:53 | constantSalt | semmle.label | constantSalt |
12+
subpaths
13+
#select
14+
| test.swift:51:49:51:49 | constantSalt | test.swift:43:35:43:130 | [...] : | test.swift:51:49:51:49 | constantSalt | The value '[...]' is used as a constant salt, which is insecure for hashing passwords. |
15+
| test.swift:56:59:56:59 | constantSalt | test.swift:43:35:43:130 | [...] : | test.swift:56:59:56:59 | constantSalt | The value '[...]' is used as a constant salt, which is insecure for hashing passwords. |
16+
| test.swift:62:59:62:59 | constantSalt | test.swift:43:35:43:130 | [...] : | test.swift:62:59:62:59 | constantSalt | The value '[...]' is used as a constant salt, which is insecure for hashing passwords. |
17+
| test.swift:67:53:67:53 | constantSalt | test.swift:43:35:43:130 | [...] : | test.swift:67:53:67:53 | constantSalt | The value '[...]' is used as a constant salt, which is insecure for hashing passwords. |

‎swift/ql/test/query-tests/Security/CWE-760/ConstantSalt.qlref

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

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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+
struct HKDF {
20+
init(password: Array<UInt8>, salt: Array<UInt8>? = nil, info: Array<UInt8>? = nil, keyLength: Int? = nil, variant: Variant = .sha2) { }
21+
}
22+
23+
final class Scrypt {
24+
init(password: Array<UInt8>, salt: Array<UInt8>, dkLen: Int, N: Int, r: Int, p: Int) { }
25+
}
26+
27+
// Helper functions
28+
func getConstantString() -> String {
29+
"this string is constant"
30+
}
31+
32+
func getConstantArray() -> Array<UInt8> {
33+
[UInt8](getConstantString().utf8)
34+
}
35+
36+
func getRandomArray() -> Array<UInt8> {
37+
(0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
38+
}
39+
40+
// --- tests ---
41+
42+
func test() {
43+
let constantSalt: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
44+
let constantStringSalt = getConstantArray()
45+
let randomSalt = getRandomArray()
46+
let randomArray = getRandomArray()
47+
let variant = Variant.sha2
48+
let iterations = 120120
49+
50+
// HKDF test cases
51+
let hkdfb1 = HKDF(password: randomArray, salt: constantSalt, info: randomArray, keyLength: 0, variant: variant) // BAD
52+
let hkdfb2 = HKDF(password: randomArray, salt: constantStringSalt, info: randomArray, keyLength: 0, variant: variant) // BAD [NOT DETECTED]
53+
let hkdfg1 = HKDF(password: randomArray, salt: randomSalt, info: randomArray, keyLength: 0, variant: variant) // GOOD
54+
55+
// PBKDF1 test cases
56+
let pbkdf1b1 = PKCS5.PBKDF1(password: randomArray, salt: constantSalt, iterations: iterations, keyLength: 0) // BAD
57+
let pbkdf1b2 = PKCS5.PBKDF1(password: randomArray, salt: constantStringSalt, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
58+
let pbkdf1g1 = PKCS5.PBKDF1(password: randomArray, salt: randomSalt, iterations: iterations, keyLength: 0) // GOOD
59+
60+
61+
// PBKDF2 test cases
62+
let pbkdf2b1 = PKCS5.PBKDF2(password: randomArray, salt: constantSalt, iterations: iterations, keyLength: 0) // BAD
63+
let pbkdf2b2 = PKCS5.PBKDF2(password: randomArray, salt: constantStringSalt, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
64+
let pbkdf2g1 = PKCS5.PBKDF2(password: randomArray, salt: randomSalt, iterations: iterations, keyLength: 0) // GOOD
65+
66+
// Scrypt test cases
67+
let scryptb1 = Scrypt(password: randomArray, salt: constantSalt, dkLen: 64, N: 16384, r: 8, p: 1) // BAD
68+
let scryptb2 = Scrypt(password: randomArray, salt: constantStringSalt, dkLen: 64, N: 16384, r: 8, p: 1) // BAD [NOT DETECTED]
69+
let scryptg1 = Scrypt(password: randomArray, salt: randomSalt, dkLen: 64, N: 16384, r: 8, p: 1) // GOOD
70+
}

0 commit comments

Comments
 (0)