Swift: detect the use of constant salts #10993

karimhamdanali · 2022-10-26T13:34:26Z

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.

The rule currently supports all ciphers that the CryptoSwift API provides, but we can always extend it further if more APIs are added.

I'd appreciate a review of the query itself, the accompanying tests, and the associated documentation.

github-actions · 2022-10-26T13:35:43Z

QHelp previews:

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

Constant salt

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.

Recommendation

Use randomly generated salts to securely hash input data.

Example

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 attakcs. In the 'GOOD' cases, the salt is randomly generated, which protects the hashed data against recovery.


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)

	// ...
}

References

What are Salted Passwords and Password Hashing?
Common Weakness Enumeration: CWE-760.

geoffw0 · 2022-10-28T09:49:07Z

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

+    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


This looks like a great start, my only concern is how often salts are specified in the wild using the functions modelled above. I did a quick MRVA query for parameters called "salt" (or similar) and the most common call that appears to be relevant is to a thing called CCKeyDerivationPBKDF(_:_:_:_:_:_:_:_:_:) from CommonCrypto. Do you think we should perhaps create a follow-up issue to add a model of that to this query as well?

add a query that detects the use of constant salts

420c35d

karimhamdanali requested a review from a team as a code owner Oct 26, 2022

github-actions bot added documentation Swift labels Oct 26, 2022

geoffw0 reviewed Oct 28, 2022

View changes

Swift: detect the use of constant salts #10993

Swift: detect the use of constant salts #10993

karimhamdanali commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

Constant salt

Recommendation

Example

References

geoffw0 Oct 28, 2022

Swift: detect the use of constant salts #10993

Are you sure you want to change the base?

Swift: detect the use of constant salts #10993

Conversation

karimhamdanali commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

Constant salt

Recommendation

Example

References

geoffw0 Oct 28, 2022

Choose a reason for hiding this comment