Swift: detect the use of constant salts

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

@@ -0,0 +1,21 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <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>
+  </overview>
+
+  <recommendation>
+    <p>Use randomly generated salts to securely hash input data.</p>
+  </recommendation>
+
+  <example>
+    <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>
+    <sample src="ConstantSalt.swift" />
+  </example>
+
+  <references>
+    <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>
+  </references>
+</qhelp>

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

@@ -0,0 +1,63 @@
+/**
+ * @name Use of constant salts
+ * @description Using constant salts for password hashing is not secure because potential attackers can precompute the hash value via dictionary attacks.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id swift/constant-salt
+ * @tags security
+ *       external/cwe/cwe-760
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.TaintTracking
+import codeql.swift.dataflow.FlowSteps
+import DataFlow::PathGraph
+
+/**
+ * A constant salt is created through either a byte array or string literals.
+ */
+class ConstantSaltSource extends Expr {
+  ConstantSaltSource() {
+    this = any(ArrayExpr arr | arr.getType().getName() = "Array<UInt8>") or
+    this instanceof StringLiteralExpr
+  }
+}
+
+/**
+ * A class for all ways to use a constant salt.
+ */
+class ConstantSaltSink extends Expr {
+  ConstantSaltSink() {
+    // `salt` arg in `init` is a sink
+    exists(ClassOrStructDecl c, AbstractFunctionDecl f, CallExpr call, int arg |
+      c.getFullName() = ["HKDF", "PBKDF1", "PBKDF2", "Scrypt"] and
+      c.getAMember() = f and
+      f.getName().matches("%init(%salt:%") and
+      call.getStaticTarget() = f and
+      f.getParam(pragma[only_bind_into](arg)).getName() = "salt" and
+      call.getArgument(pragma[only_bind_into](arg)).getExpr() = this
+    )
+  }
+}
+
+/**
+ * A taint configuration from the source of constants salts to expressions that use
+ * them to initialize password-based enecryption keys.
+ */
+class ConstantSaltConfig extends TaintTracking::Configuration {
+  ConstantSaltConfig() { this = "ConstantSaltConfig" }
+
+  override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSource }
+
+  override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSink }
+}
+
+// The query itself
+from ConstantSaltConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
+where config.hasFlowPath(sourceNode, sinkNode)
+select sinkNode.getNode(), sourceNode, sinkNode,
+  "The value '" + sourceNode.getNode().toString() +
+    "' is used as a constant salt, which is insecure for hashing passwords."

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

@@ -0,0 +1,22 @@
+
+func encrypt(padding : Padding) {
+	// ...
+
+	// BAD: Using constant salts for hashing
+	let salt: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05]
+	let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
+	_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
+	_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
+	_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
+	_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
+
+	// GOOD: Using randomly generated salts for hashing
+	let salt = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
+	let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
+	_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
+	_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
+	_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
+	_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
+
+	// ...
+}

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

@@ -0,0 +1,17 @@
+edges
+| test.swift:43:35:43:130 | [...] :  | test.swift:51:49:51:49 | constantSalt |
+| test.swift:43:35:43:130 | [...] :  | test.swift:56:59:56:59 | constantSalt |
+| test.swift:43:35:43:130 | [...] :  | test.swift:62:59:62:59 | constantSalt |
+| test.swift:43:35:43:130 | [...] :  | test.swift:67:53:67:53 | constantSalt |
+nodes
+| test.swift:43:35:43:130 | [...] :  | semmle.label | [...] :  |
+| test.swift:51:49:51:49 | constantSalt | semmle.label | constantSalt |
+| test.swift:56:59:56:59 | constantSalt | semmle.label | constantSalt |
+| test.swift:62:59:62:59 | constantSalt | semmle.label | constantSalt |
+| test.swift:67:53:67:53 | constantSalt | semmle.label | constantSalt |
+subpaths
+#select
+| 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. |
+| 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. |
+| 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. |
+| 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

@@ -0,0 +1 @@
+queries/Security/CWE-760/ConstantSalt.ql

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

@@ -0,0 +1,70 @@
+
+// --- stubs ---
+
+// These stubs roughly follows the same structure as classes from CryptoSwift
+enum PKCS5 { }
+
+enum Variant { case md5, sha1, sha2, sha3 }
+
+extension PKCS5 {
+  struct PBKDF1 {
+	init(password: Array<UInt8>, salt: Array<UInt8>, variant: Variant = .sha1, iterations: Int = 4096, keyLength: Int? = nil) { }
+  }
+
+  struct PBKDF2 {
+	init(password: Array<UInt8>, salt: Array<UInt8>, iterations: Int = 4096, keyLength: Int? = nil, variant: Variant = .sha2) { }
+  }
+}
+
+struct HKDF {
+	init(password: Array<UInt8>, salt: Array<UInt8>? = nil, info: Array<UInt8>? = nil, keyLength: Int? = nil, variant: Variant = .sha2) { }
+}
+
+final class Scrypt {
+	init(password: Array<UInt8>, salt: Array<UInt8>, dkLen: Int, N: Int, r: Int, p: Int) { }
+}
+
+// Helper functions
+func getConstantString() -> String {
+  "this string is constant"
+}
+
+func getConstantArray() -> Array<UInt8> {
+	[UInt8](getConstantString().utf8)
+}
+
+func getRandomArray() -> Array<UInt8> {
+	(0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
+}
+
+// --- tests ---
+
+func test() {
+	let constantSalt: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
+	let constantStringSalt = getConstantArray()
+	let randomSalt = getRandomArray()
+	let randomArray = getRandomArray()
+	let variant = Variant.sha2
+	let iterations = 120120
+
+	// HKDF test cases
+	let hkdfb1 = HKDF(password: randomArray, salt: constantSalt, info: randomArray, keyLength: 0, variant: variant) // BAD
+	let hkdfb2 = HKDF(password: randomArray, salt: constantStringSalt, info: randomArray, keyLength: 0, variant: variant) // BAD [NOT DETECTED]
+	let hkdfg1 = HKDF(password: randomArray, salt: randomSalt, info: randomArray, keyLength: 0, variant: variant) // GOOD
+
+	// PBKDF1 test cases
+	let pbkdf1b1 = PKCS5.PBKDF1(password: randomArray, salt: constantSalt, iterations: iterations, keyLength: 0) // BAD
+	let pbkdf1b2 = PKCS5.PBKDF1(password: randomArray, salt: constantStringSalt, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
+	let pbkdf1g1 = PKCS5.PBKDF1(password: randomArray, salt: randomSalt, iterations: iterations, keyLength: 0) // GOOD
+
+
+	// PBKDF2 test cases
+	let pbkdf2b1 = PKCS5.PBKDF2(password: randomArray, salt: constantSalt, iterations: iterations, keyLength: 0) // BAD
+	let pbkdf2b2 = PKCS5.PBKDF2(password: randomArray, salt: constantStringSalt, iterations: iterations, keyLength: 0) // BAD [NOT DETECTED]
+	let pbkdf2g1 = PKCS5.PBKDF2(password: randomArray, salt: randomSalt, iterations: iterations, keyLength: 0) // GOOD
+
+	// Scrypt test cases
+	let scryptb1 = Scrypt(password: randomArray, salt: constantSalt, dkLen: 64, N: 16384, r: 8, p: 1) // BAD
+	let scryptb2 = Scrypt(password: randomArray, salt: constantStringSalt, dkLen: 64, N: 16384, r: 8, p: 1) // BAD [NOT DETECTED]
+	let scryptg1 = Scrypt(password: randomArray, salt: randomSalt, dkLen: 64, N: 16384, r: 8, p: 1) // GOOD
+}