Skip to content

Commit 19b13ee

Browse files
redsun82redsun82
committed
Swift: first draft of query targeting weak hashing
1 parent 95a9faf commit 19b13ee

File tree

8 files changed

+273
-8
lines changed

8 files changed

+273
-8
lines changed

‎swift/ql/lib/codeql/swift/elements/AstNode.qll

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,30 @@
11
private import codeql.swift.generated.AstNode
22
private import codeql.swift.elements.decl.AbstractFunctionDecl
3+
private import codeql.swift.elements.decl.Decl
34
private import codeql.swift.generated.ParentChild
45

5-
private Element getEnclosingFunctionStep(Element e) {
6-
not e instanceof AbstractFunctionDecl and
7-
result = getImmediateParent(e)
8-
}
6+
private module Cached {
7+
private Element getEnclosingDeclStep(Element e) {
8+
not e instanceof Decl and
9+
result = getImmediateParent(e)
10+
}
11+
12+
cached
13+
Decl getEnclosingDecl(AstNode ast) { result = getEnclosingDeclStep*(getImmediateParent(ast)) }
914

10-
cached
11-
private AbstractFunctionDecl getEnclosingFunctionCached(AstNode ast) {
12-
result = getEnclosingFunctionStep*(getImmediateParent(ast))
15+
private Element getEnclosingFunctionStep(Element e) {
16+
not e instanceof AbstractFunctionDecl and
17+
result = getEnclosingDecl(e)
18+
}
19+
20+
cached
21+
AbstractFunctionDecl getEnclosingFunction(AstNode ast) {
22+
result = getEnclosingFunctionStep*(getEnclosingDecl(ast))
23+
}
1324
}
1425

1526
class AstNode extends AstNodeBase {
16-
final AbstractFunctionDecl getEnclosingFunction() { result = getEnclosingFunctionCached(this) }
27+
final AbstractFunctionDecl getEnclosingFunction() { result = Cached::getEnclosingFunction(this) }
28+
29+
final Decl getEnclosingDecl() { result = Cached::getEnclosingDecl(this) }
1730
}

‎swift/ql/src/queries/Security/CWE-328/WeakSensitiveDataHashing.qhelp

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Using a broken or weak cryptographic hash function can leave data
8+
vulnerable, and should not be used in security related code.
9+
</p>
10+
11+
<p>
12+
A strong cryptographic hash function should be resistant to:
13+
</p>
14+
<ul>
15+
<li>
16+
pre-image attacks: if you know a hash value <code>h(x)</code>,
17+
you should not be able to easily find the input <code>x</code>.
18+
</li>
19+
<li>
20+
collision attacks: if you know a hash value <code>h(x)</code>,
21+
you should not be able to easily find a different input
22+
<code>y</code>
23+
with the same hash value <code>h(x) = h(y)</code>.
24+
</li>
25+
</ul>
26+
<!--<p>
27+
In cases with a limited input space, such as for passwords, the hash
28+
function also needs to be computationally expensive to be resistant to
29+
brute-force attacks. Passwords should also have an unique salt applied
30+
before hashing, but that is not considered by this query.
31+
</p>-->
32+
33+
<p>
34+
As an example, both MD5 and SHA-1 are known to be vulnerable to collision attacks.
35+
</p>
36+
37+
<p>
38+
Since it's OK to use a weak cryptographic hash function in a non-security
39+
context, this query only alerts when these are used to hash sensitive
40+
data (such as passwords, certificates, usernames).
41+
</p>
42+
43+
</overview>
44+
<recommendation>
45+
46+
<p>
47+
Ensure that you use a strong, modern cryptographic hash function:
48+
</p>
49+
50+
<ul>
51+
<li>
52+
such as Argon2, scrypt, bcrypt, or PBKDF2 for passwords and other data with limited input space.
53+
</li>
54+
<li>
55+
such as SHA-2, or SHA-3 in other cases.
56+
</li>
57+
</ul>
58+
59+
</recommendation>
60+
<example>
61+
62+
<p>
63+
The following examples show a function for checking whether the hash
64+
of a certificate matches a known value -- to prevent tampering.
65+
66+
In the first case the MD5 hashing algorithm is used that is known to be vulnerable to collision attacks.
67+
</p>
68+
<sample src="WeakSensitiveDataHashingBad.swift"/>
69+
<p>
70+
71+
Here is the same function using SHA-512 that is a strong cryptographic hashing function.
72+
</p>
73+
<sample src="WeakSensitiveDataHashingGood.swift"/>
74+
75+
</example>
76+
<references>
77+
<li>
78+
OWASP:
79+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage
80+
Cheat Sheet
81+
</a>
82+
and
83+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Protection_Cheat_Sheet.html#use-strong-cryptographic-hashing-algorithms">
84+
Transport Layer Protection Cheat Sheet
85+
</a>
86+
</li>
87+
</references>
88+
89+
</qhelp>

‎swift/ql/src/queries/Security/CWE-328/WeakSensitiveDataHashing.ql

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* @name Use of a broken or weak cryptographic hashing algorithm on sensitive data
3+
* @description Using broken or weak cryptographic hashing algorithms can compromise security.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id py/weak-sensitive-data-hashing
9+
* @tags security
10+
* external/cwe/cwe-327
11+
* external/cwe/cwe-328
12+
*/
13+
14+
import swift
15+
import codeql.swift.security.SensitiveExprs
16+
import codeql.swift.dataflow.DataFlow
17+
import codeql.swift.dataflow.TaintTracking
18+
import DataFlow::PathGraph
19+
20+
class WeakHashingConfig extends TaintTracking::Configuration {
21+
WeakHashingConfig() { this = "WeakHashingConfig" }
22+
23+
override predicate isSource(DataFlow::Node node) {
24+
exists(SensitiveExpr e |
25+
node.asExpr() = e and
26+
not e.isProbablySafe()
27+
)
28+
}
29+
30+
override predicate isSink(DataFlow::Node node) { node instanceof WeakHashingConfig::Sink }
31+
}
32+
33+
module WeakHashingConfig {
34+
class Sink extends DataFlow::Node {
35+
string algorithm;
36+
37+
Sink() {
38+
exists(ApplyExpr call, FuncDecl func |
39+
call.getAnArgument().getExpr() = this.asExpr() and
40+
call.getStaticTarget() = func and
41+
func.getName().matches(["hash(%", "update(%"]) and
42+
algorithm = func.getEnclosingDecl().(StructDecl).getName() and
43+
algorithm = ["MD5", "SHA1"]
44+
)
45+
}
46+
47+
string getAlgorithm() { result = algorithm }
48+
}
49+
}
50+
51+
from WeakHashingConfig config, DataFlow::PathNode source, DataFlow::PathNode sink, string algorithm
52+
where
53+
config.hasFlowPath(source, sink) and
54+
algorithm = sink.getNode().(WeakHashingConfig::Sink).getAlgorithm()
55+
select sink.getNode(), source, sink,
56+
"Insecure hashing algorithm (" + algorithm + ") depends on $@.", source.getNode(),
57+
"sensitive data"

‎swift/ql/src/queries/Security/CWE-328/WeakSensitiveDataHashingBad.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
typealias Hasher = Crypto.Insecure.MD5
2+
3+
func checkCertificate(cert: Array[UInt8], hash: Array[UInt8]) -> Bool
4+
return Hasher.hash(data: cert) == hash // BAD
5+
}

‎swift/ql/src/queries/Security/CWE-328/WeakSensitiveDataHashingGood.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
typealias Hasher = Crypto.SHA512
2+
3+
func checkCertificate(cert: Array[UInt8], hash: Array[UInt8]) -> Bool
4+
return Hasher.hash(data: cert) == hash // GOOD

‎swift/ql/test/query-tests/Security/CWE-328/WeakSensitiveDataHashing.expected

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
edges
2+
nodes
3+
| testCrypto.swift:25:47:25:47 | passwd | semmle.label | passwd |
4+
| testCrypto.swift:28:43:28:43 | credit_card_no | semmle.label | credit_card_no |
5+
| testCrypto.swift:32:48:32:48 | passwd | semmle.label | passwd |
6+
| testCrypto.swift:35:44:35:44 | credit_card_no | semmle.label | credit_card_no |
7+
| testCrypto.swift:40:23:40:23 | passwd | semmle.label | passwd |
8+
| testCrypto.swift:43:23:43:23 | credit_card_no | semmle.label | credit_card_no |
9+
| testCrypto.swift:48:23:48:23 | passwd | semmle.label | passwd |
10+
| testCrypto.swift:51:23:51:23 | credit_card_no | semmle.label | credit_card_no |
11+
| testCrypto.swift:56:32:56:32 | passwd | semmle.label | passwd |
12+
| testCrypto.swift:59:32:59:32 | credit_card_no | semmle.label | credit_card_no |
13+
| testCrypto.swift:64:32:64:32 | passwd | semmle.label | passwd |
14+
| testCrypto.swift:67:32:67:32 | credit_card_no | semmle.label | credit_card_no |
15+
subpaths
16+
#select
17+
| testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | testCrypto.swift:25:47:25:47 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:25:47:25:47 | passwd | sensitive data |
18+
| testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | testCrypto.swift:28:43:28:43 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:28:43:28:43 | credit_card_no | sensitive data |
19+
| testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | testCrypto.swift:32:48:32:48 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:32:48:32:48 | passwd | sensitive data |
20+
| testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | testCrypto.swift:35:44:35:44 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:35:44:35:44 | credit_card_no | sensitive data |
21+
| testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | testCrypto.swift:40:23:40:23 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:40:23:40:23 | passwd | sensitive data |
22+
| testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | testCrypto.swift:43:23:43:23 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:43:23:43:23 | credit_card_no | sensitive data |
23+
| testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | testCrypto.swift:48:23:48:23 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:48:23:48:23 | passwd | sensitive data |
24+
| testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | testCrypto.swift:51:23:51:23 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:51:23:51:23 | credit_card_no | sensitive data |
25+
| testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | testCrypto.swift:56:32:56:32 | passwd | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:56:32:56:32 | passwd | sensitive data |
26+
| testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | testCrypto.swift:59:32:59:32 | credit_card_no | Insecure hashing algorithm (MD5) depends on $@. | testCrypto.swift:59:32:59:32 | credit_card_no | sensitive data |
27+
| testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | testCrypto.swift:64:32:64:32 | passwd | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:64:32:64:32 | passwd | sensitive data |
28+
| testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | testCrypto.swift:67:32:67:32 | credit_card_no | Insecure hashing algorithm (SHA1) depends on $@. | testCrypto.swift:67:32:67:32 | credit_card_no | sensitive data |

‎swift/ql/test/query-tests/Security/CWE-328/WeakSensitiveDataHashing.qlref

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

‎swift/ql/test/query-tests/Security/CWE-328/testCrypto.swift

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
//codeql-extractor-options: -module-name Crypto
2+
3+
enum Insecure {
4+
struct MD5 {
5+
static func hash<D>(data: D) -> [UInt8] {
6+
return []
7+
}
8+
9+
func update<D>(data: D) {}
10+
func update(bufferPointer: UnsafeRawBufferPointer) {}
11+
func finalize() -> [UInt8] { return [] }
12+
}
13+
struct SHA1 {
14+
static func hash<D>(data: D) -> [UInt8] {
15+
return []
16+
}
17+
18+
func update<D>(data: D) {}
19+
func update(bufferPointer: UnsafeRawBufferPointer) {}
20+
func finalize() -> [UInt8] { return [] }
21+
}
22+
}
23+
24+
func test1(passwd : UnsafeRawBufferPointer, encrypted_passwd : String, account_no : String, credit_card_no : String) {
25+
var hash = Crypto.Insecure.MD5.hash(data: passwd) // BAD
26+
hash = Crypto.Insecure.MD5.hash(data: encrypted_passwd) // GOOD (not sensitive)
27+
hash = Crypto.Insecure.MD5.hash(data: account_no) // BAD [NOT DETECTED]
28+
hash = Crypto.Insecure.MD5.hash(data: credit_card_no) // BAD
29+
}
30+
31+
func test2(passwd : String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
32+
var hash = Crypto.Insecure.SHA1.hash(data: passwd) // BAD
33+
hash = Crypto.Insecure.SHA1.hash(data: encrypted_passwd) // GOOD (not sensitive)
34+
hash = Crypto.Insecure.SHA1.hash(data: account_no) // BAD [NOT DETECTED]
35+
hash = Crypto.Insecure.SHA1.hash(data: credit_card_no) // BAD
36+
}
37+
38+
func test3(passwd : String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
39+
var hash = Crypto.Insecure.MD5()
40+
hash.update(data: passwd) // BAD
41+
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
42+
hash.update(data: account_no) // BAD [NOT DETECTED]
43+
hash.update(data: credit_card_no) // BAD
44+
}
45+
46+
func test4(passwd : String, encrypted_passwd : String, account_no : String, credit_card_no : String) {
47+
var hash = Crypto.Insecure.SHA1()
48+
hash.update(data: passwd) // BAD
49+
hash.update(data: encrypted_passwd) // GOOD (not sensitive)
50+
hash.update(data: account_no) // BAD [NOT DETECTED]
51+
hash.update(data: credit_card_no) // BAD
52+
}
53+
54+
func test5(passwd : UnsafeRawBufferPointer, encrypted_passwd : UnsafeRawBufferPointer, account_no : UnsafeRawBufferPointer, credit_card_no : UnsafeRawBufferPointer) {
55+
var hash = Crypto.Insecure.MD5()
56+
hash.update(bufferPointer: passwd) // BAD
57+
hash.update(bufferPointer: encrypted_passwd) // GOOD (not sensitive)
58+
hash.update(bufferPointer: account_no) // BAD [NOT DETECTED]
59+
hash.update(bufferPointer: credit_card_no) // BAD
60+
}
61+
62+
func test6(passwd : UnsafeRawBufferPointer, encrypted_passwd : UnsafeRawBufferPointer, account_no : UnsafeRawBufferPointer, credit_card_no : UnsafeRawBufferPointer) {
63+
var hash = Crypto.Insecure.SHA1()
64+
hash.update(bufferPointer: passwd) // BAD
65+
hash.update(bufferPointer: encrypted_passwd) // GOOD (not sensitive)
66+
hash.update(bufferPointer: account_no) // BAD [NOT DETECTED]
67+
hash.update(bufferPointer: credit_card_no) // BAD
68+
}

0 commit comments

Comments
 (0)