Skip to content

Commit b209cac

Browse files
karimhamdanalikarimhamdanali
authored
Merge pull request github#11063 from karimhamdanali/swift-pbe-constant-password
Swift: detect the use of constant passwords for password-based encryption
2 parents e18b2cf + 7d473fb commit b209cac

File tree

6 files changed

+197
-0
lines changed

6 files changed

+197
-0
lines changed

‎swift/ql/src/queries/Security/CWE-259/ConstantPassword.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>Deriving password-based encryption keys using hardcoded passwords is insecure, because the generated key may be easily discovered. Data hashed using constant salts is vulnerable to dictionary attacks, enabling attackers to recover the original input.</p>
7+
<p>In particular, constant passwords would enable easier recovery of the key, even in the presence of a salt. If that salt is random enough, then key recovery is not as easy as just looking up a hardcoded credential in the source code.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use randomly generated passwords to securely derive a password-based encryption key.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>The following example shows a few cases of hashing input data. In the 'BAD' cases, the password is constant, making the derived key vulnerable to dictionary attakcs. In the 'GOOD' cases, the password is randomly generated, which protects the hashed data against recovery.</p>
16+
<sample src="ConstantPassword.swift" />
17+
</example>
18+
19+
<references>
20+
<li>Okta blog: <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>
21+
<li>RFC 2898: <a href="https://www.rfc-editor.org/rfc/rfc2898">Password-Based Cryptography Specification</a>.</li>
22+
</references>
23+
</qhelp>

‎swift/ql/src/queries/Security/CWE-259/ConstantPassword.ql

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* @name Constant password
3+
* @description Using constant passwords is not secure, because potential attackers can easily recover them from the source code.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 6.8
7+
* @precision high
8+
* @id swift/constant-password
9+
* @tags security
10+
* external/cwe/cwe-259
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 password is created through either a byte array or string literals.
21+
*/
22+
class ConstantPasswordSource extends Expr {
23+
ConstantPasswordSource() {
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 password.
31+
*/
32+
class ConstantPasswordSink extends Expr {
33+
ConstantPasswordSink() {
34+
// `password` 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(%password:%") and
39+
call.getStaticTarget() = f and
40+
f.getParam(pragma[only_bind_into](arg)).getName() = "password" 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 passwords to expressions that use
48+
* them to initialize password-based encryption keys.
49+
*/
50+
class ConstantPasswordConfig extends TaintTracking::Configuration {
51+
ConstantPasswordConfig() { this = "ConstantPasswordConfig" }
52+
53+
override predicate isSource(DataFlow::Node node) {
54+
node.asExpr() instanceof ConstantPasswordSource
55+
}
56+
57+
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof ConstantPasswordSink }
58+
}
59+
60+
// The query itself
61+
from ConstantPasswordConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
62+
where config.hasFlowPath(sourceNode, sinkNode)
63+
select sinkNode.getNode(), sourceNode, sinkNode,
64+
"The value '" + sourceNode.getNode().toString() + "' is used as a constant password."

‎swift/ql/src/queries/Security/CWE-259/ConstantPassword.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 passwords for hashing
6+
let password: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05]
7+
let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
8+
_ = try HKDF(password: password, salt: randomArray, info: randomArray, keyLength: 0, variant: Variant.sha2)
9+
_ = try PKCS5.PBKDF1(password: password, salt: randomArray, iterations: 120120, keyLength: 0)
10+
_ = try PKCS5.PBKDF2(password: password, salt: randomArray, iterations: 120120, keyLength: 0)
11+
_ = try Scrypt(password: password, salt: randomArray, dkLen: 64, N: 16384, r: 8, p: 1)
12+
13+
// GOOD: Using randomly generated passwords for hashing
14+
let password = (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: password, salt: randomArray, info: randomArray, keyLength: 0, variant: Variant.sha2)
17+
_ = try PKCS5.PBKDF1(password: password, salt: randomArray, iterations: 120120, keyLength: 0)
18+
_ = try PKCS5.PBKDF2(password: password, salt: randomArray, iterations: 120120, keyLength: 0)
19+
_ = try Scrypt(password: password, salt: randomArray, dkLen: 64, N: 16384, r: 8, p: 1)
20+
21+
// ...
22+
}

‎swift/ql/test/query-tests/Security/CWE-259/ConstantPassword.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:39:43:134 | [...] : | test.swift:51:30:51:30 | constantPassword |
3+
| test.swift:43:39:43:134 | [...] : | test.swift:56:40:56:40 | constantPassword |
4+
| test.swift:43:39:43:134 | [...] : | test.swift:62:40:62:40 | constantPassword |
5+
| test.swift:43:39:43:134 | [...] : | test.swift:67:34:67:34 | constantPassword |
6+
nodes
7+
| test.swift:43:39:43:134 | [...] : | semmle.label | [...] : |
8+
| test.swift:51:30:51:30 | constantPassword | semmle.label | constantPassword |
9+
| test.swift:56:40:56:40 | constantPassword | semmle.label | constantPassword |
10+
| test.swift:62:40:62:40 | constantPassword | semmle.label | constantPassword |
11+
| test.swift:67:34:67:34 | constantPassword | semmle.label | constantPassword |
12+
subpaths
13+
#select
14+
| test.swift:51:30:51:30 | constantPassword | test.swift:43:39:43:134 | [...] : | test.swift:51:30:51:30 | constantPassword | The value '[...]' is used as a constant password. |
15+
| test.swift:56:40:56:40 | constantPassword | test.swift:43:39:43:134 | [...] : | test.swift:56:40:56:40 | constantPassword | The value '[...]' is used as a constant password. |
16+
| test.swift:62:40:62:40 | constantPassword | test.swift:43:39:43:134 | [...] : | test.swift:62:40:62:40 | constantPassword | The value '[...]' is used as a constant password. |
17+
| test.swift:67:34:67:34 | constantPassword | test.swift:43:39:43:134 | [...] : | test.swift:67:34:67:34 | constantPassword | The value '[...]' is used as a constant password. |

‎swift/ql/test/query-tests/Security/CWE-259/ConstantPassword.qlref

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

‎swift/ql/test/query-tests/Security/CWE-259/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 constantPassword: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
44+
let constantStringPassword = getConstantArray()
45+
let randomPassword = getRandomArray()
46+
let randomArray = getRandomArray()
47+
let variant = Variant.sha2
48+
let iterations = 120120
49+
50+
// HKDF test cases
51+
let hkdfb1 = HKDF(password: constantPassword, salt: randomArray, info: randomArray, keyLength: 0, variant: variant) // BAD
52+
let hkdfb2 = HKDF(password: constantStringPassword, salt: randomArray, info: randomArray, keyLength: 0, variant: variant) // BAD [NOT DETECTED]
53+
let hkdfg1 = HKDF(password: randomPassword, salt: randomArray, info: randomArray, keyLength: 0, variant: variant) // GOOD
54+
55+
// PBKDF1 test cases
56+
let pbkdf1b1 = PKCS5.PBKDF1(password: constantPassword, salt: randomArray, iterations: iterations, keyLength: 0) // BAD
57+
let pbkdf1b2 = PKCS5.PBKDF1(password: constantStringPassword, salt: randomArray, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
58+
let pbkdf1g1 = PKCS5.PBKDF1(password: randomPassword, salt: randomArray, iterations: iterations, keyLength: 0) // GOOD
59+
60+
61+
// PBKDF2 test cases
62+
let pbkdf2b1 = PKCS5.PBKDF2(password: constantPassword, salt: randomArray, iterations: iterations, keyLength: 0) // BAD
63+
let pbkdf2b2 = PKCS5.PBKDF2(password: constantStringPassword, salt: randomArray, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
64+
let pbkdf2g1 = PKCS5.PBKDF2(password: randomPassword, salt: randomArray, iterations: iterations, keyLength: 0) // GOOD
65+
66+
// Scrypt test cases
67+
let scryptb1 = Scrypt(password: constantPassword, salt: randomArray, dkLen: 64, N: 16384, r: 8, p: 1) // BAD
68+
let scryptb2 = Scrypt(password: constantStringPassword, salt: randomArray, dkLen: 64, N: 16384, r: 8, p: 1) // BAD [NOT DETECTED]
69+
let scryptg1 = Scrypt(password: randomPassword, salt: randomArray, dkLen: 64, N: 16384, r: 8, p: 1) // GOOD
70+
}

0 commit comments

Comments
 (0)