Skip to content

Commit 746f535

Browse files
karimhamdanalikarimhamdanali
committed
Add rule that checks for using the insecure ECB block mode for encryption
1 parent 17e6b2a commit 746f535

File tree

7 files changed

+227
-0
lines changed

7 files changed

+227
-0
lines changed

‎swift/ql/src/queries/Security/ECB-Encryption/ECBEncryption.qhelp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>ECB should not be used as a mode for encryption. It has dangerous weaknesses. Data is encrypted the same way every time
7+
meaning the same plaintext input will always produce the same ciphertext. This behaviour makes encrypted messages vulnerable
8+
to replay attacks.</p>
9+
10+
</overview>
11+
<recommendation>
12+
<p>Use a different cipher mode such as CBC.</p>
13+
14+
</recommendation>
15+
<references>
16+
17+
<li>Wikipedia, block cipher modes of operation, <a href="https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Electronic_codebook_.28ECB.29">Electronic codebook (ECB)</a>.</li>
18+
19+
</references>
20+
</qhelp>

‎swift/ql/src/queries/Security/ECB-Encryption/ECBEncryption.ql

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @name Encryption using ECB
3+
* @description Highlights uses of the encryption mode 'ECB'. This mode should normally not be used because it is vulnerable to replay attacks.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id swift/ecb-encryption
9+
* @tags security
10+
* external/cwe/cwe-327
11+
*/
12+
13+
import swift
14+
import codeql.swift.dataflow.DataFlow
15+
import codeql.swift.dataflow.TaintTracking
16+
import DataFlow::PathGraph
17+
18+
/**
19+
* An `Expr` that is used to initialize the block mode of a cipher.
20+
*/
21+
abstract class BlockMode extends Expr { }
22+
23+
/**
24+
* An `Expr` that is used to form an `AES` cipher.
25+
*/
26+
class AES extends BlockMode {
27+
AES() {
28+
// `blockMode` arg in `AES.init` is a sink
29+
exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
30+
c.getName() = "AES" and
31+
c.getAMember() = f and
32+
f.getName() = ["init(key:blockMode:)", "init(key:blockMode:padding:)"] and
33+
call.getStaticTarget() = f and
34+
call.getArgument(1).getExpr() = this
35+
)
36+
}
37+
}
38+
39+
/**
40+
* An `Expr` that is used to form a `Blowfish` cipher.
41+
*/
42+
class Blowfish extends BlockMode {
43+
Blowfish() {
44+
// `blockMode` arg in `Blowfish.init` is a sink
45+
exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
46+
c.getName() = "Blowfish" and
47+
c.getAMember() = f and
48+
f.getName() = "init(key:blockMode:padding:)" and
49+
call.getStaticTarget() = f and
50+
call.getArgument(1).getExpr() = this
51+
)
52+
}
53+
}
54+
55+
/**
56+
* A taint configuration from the constructor of ECB mode to expressions that use
57+
* it to initialize a cipher.
58+
*/
59+
class ECBEncryptionConfig extends TaintTracking::Configuration {
60+
ECBEncryptionConfig() { this = "ECBEncryptionConfig" }
61+
62+
override predicate isSource(DataFlow::Node node) {
63+
exists(StructDecl s, AbstractFunctionDecl f, CallExpr call |
64+
s.getName() = "ECB" and
65+
s.getAMember() = f and
66+
f.getName() = "init()" and
67+
call.getStaticTarget() = f and
68+
node.asExpr() = call
69+
)
70+
}
71+
72+
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof BlockMode }
73+
}
74+
75+
// The query itself
76+
from ECBEncryptionConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
77+
where config.hasFlowPath(sourceNode, sinkNode)
78+
select sinkNode.getNode(), sourceNode, sinkNode,
79+
"The initialization of the cipher '" + sinkNode.getNode().toString() +
80+
"' uses the insecure ECB block mode from $@", sourceNode,
81+
sourceNode.getNode().toString()

‎swift/ql/src/queries/Security/ECB-Encryption/ECBEncryption.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
2+
func encrypt(key : Key, padding : Padding) {
3+
// ...
4+
5+
// BAD: ECB is used for block mode
6+
let blockMode = ECB()
7+
_ = try AES(key: key, blockMode: blockMode, padding: padding)
8+
_ = try AES(key: key, blockMode: blockMode)
9+
_ = try Blowfish(key: key, blockMode: blockMode, padding: padding)
10+
11+
// GOOD: ECB is not used for block mode
12+
let blockMode = CBC()
13+
_ = try AES(key: key, blockMode: blockMode, padding: padding)
14+
_ = try AES(key: key, blockMode: blockMode)
15+
_ = try Blowfish(key: key, blockMode: blockMode, padding: padding)
16+
17+
// ...
18+
}

‎swift/ql/test/query-tests/Security/ECB-Encryption/ECBEncryption.expected

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
edges
2+
| testAES.swift:32:12:32:16 | call to init() : | testAES.swift:36:36:36:36 | ecb |
3+
| testAES.swift:32:12:32:16 | call to init() : | testAES.swift:37:36:37:36 | ecb |
4+
| testBlowfish.swift:32:12:32:16 | call to init() : | testBlowfish.swift:36:41:36:41 | ecb |
5+
nodes
6+
| testAES.swift:32:12:32:16 | call to init() : | semmle.label | call to init() : |
7+
| testAES.swift:36:36:36:36 | ecb | semmle.label | ecb |
8+
| testAES.swift:37:36:37:36 | ecb | semmle.label | ecb |
9+
| testAES.swift:38:36:38:40 | call to init() | semmle.label | call to init() |
10+
| testAES.swift:39:36:39:40 | call to init() | semmle.label | call to init() |
11+
| testBlowfish.swift:32:12:32:16 | call to init() : | semmle.label | call to init() : |
12+
| testBlowfish.swift:36:41:36:41 | ecb | semmle.label | ecb |
13+
| testBlowfish.swift:37:41:37:45 | call to init() | semmle.label | call to init() |
14+
subpaths
15+
#select
16+
| testAES.swift:36:36:36:36 | ecb | testAES.swift:32:12:32:16 | call to init() : | testAES.swift:36:36:36:36 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@ | testAES.swift:32:12:32:16 | call to init() : | call to init() |
17+
| testAES.swift:37:36:37:36 | ecb | testAES.swift:32:12:32:16 | call to init() : | testAES.swift:37:36:37:36 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@ | testAES.swift:32:12:32:16 | call to init() : | call to init() |
18+
| testAES.swift:38:36:38:40 | call to init() | testAES.swift:38:36:38:40 | call to init() | testAES.swift:38:36:38:40 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@ | testAES.swift:38:36:38:40 | call to init() | call to init() |
19+
| testAES.swift:39:36:39:40 | call to init() | testAES.swift:39:36:39:40 | call to init() | testAES.swift:39:36:39:40 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@ | testAES.swift:39:36:39:40 | call to init() | call to init() |
20+
| testBlowfish.swift:36:41:36:41 | ecb | testBlowfish.swift:32:12:32:16 | call to init() : | testBlowfish.swift:36:41:36:41 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@ | testBlowfish.swift:32:12:32:16 | call to init() : | call to init() |
21+
| testBlowfish.swift:37:41:37:45 | call to init() | testBlowfish.swift:37:41:37:45 | call to init() | testBlowfish.swift:37:41:37:45 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@ | testBlowfish.swift:37:41:37:45 | call to init() | call to init() |

‎swift/ql/test/query-tests/Security/ECB-Encryption/ECBEncryption.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/Security/ECB-Encryption/ECBEncryption.ql

‎swift/ql/test/query-tests/Security/ECB-Encryption/testAES.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
2+
// --- stubs ---
3+
4+
// These stubs roughly follows the same structure as classes from CryptoSwift
5+
class AES
6+
{
7+
init(key: Array<UInt8>, blockMode: BlockMode, padding: Padding) { }
8+
init(key: Array<UInt8>, blockMode: BlockMode) { }
9+
}
10+
11+
protocol BlockMode { }
12+
13+
struct ECB: BlockMode {
14+
init() { }
15+
}
16+
17+
struct CBC: BlockMode {
18+
init() { }
19+
}
20+
21+
protocol PaddingProtocol { }
22+
23+
enum Padding: PaddingProtocol {
24+
case noPadding, zeroPadding, pkcs7, pkcs5, eme_pkcs1v15, emsa_pkcs1v15, iso78164, iso10126
25+
}
26+
27+
28+
// --- tests ---
29+
30+
func test1() {
31+
let key: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
32+
let ecb = ECB()
33+
let cbc = CBC()
34+
let padding = Padding.noPadding
35+
36+
let b1 = AES(key: key, blockMode: ecb, padding: padding) // BAD
37+
let b2 = AES(key: key, blockMode: ecb) // BAD
38+
let b3 = AES(key: key, blockMode: ECB(), padding: padding) // BAD
39+
let b4 = AES(key: key, blockMode: ECB()) // BAD
40+
41+
let g1 = AES(key: key, blockMode: cbc, padding: padding) // GOOD
42+
let g2 = AES(key: key, blockMode: cbc) // GOOD
43+
let g3 = AES(key: key, blockMode: CBC(), padding: padding) // GOOD
44+
let g4 = AES(key: key, blockMode: CBC()) // GOOD
45+
}

‎swift/ql/test/query-tests/Security/ECB-Encryption/testBlowfish.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
2+
// --- stubs ---
3+
4+
// These stubs roughly follows the same structure as classes from CryptoSwift
5+
class Blowfish
6+
{
7+
init(key: Array<UInt8>, blockMode: BlockMode, padding: Padding) { }
8+
init(key: Array<UInt8>, blockMode: BlockMode) { }
9+
}
10+
11+
protocol BlockMode { }
12+
13+
struct ECB: BlockMode {
14+
init() { }
15+
}
16+
17+
struct CBC: BlockMode {
18+
init() { }
19+
}
20+
21+
protocol PaddingProtocol { }
22+
23+
enum Padding: PaddingProtocol {
24+
case noPadding, zeroPadding, pkcs7, pkcs5, eme_pkcs1v15, emsa_pkcs1v15, iso78164, iso10126
25+
}
26+
27+
28+
// --- tests ---
29+
30+
func test1() {
31+
let key: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
32+
let ecb = ECB()
33+
let cbc = CBC()
34+
let padding = Padding.noPadding
35+
36+
let b1 = Blowfish(key: key, blockMode: ecb, padding: padding) // BAD
37+
let b2 = Blowfish(key: key, blockMode: ECB(), padding: padding) // BAD
38+
39+
let g1 = Blowfish(key: key, blockMode: cbc, padding: padding) // GOOD
40+
let g2 = Blowfish(key: key, blockMode: CBC(), padding: padding) // GOOD
41+
}

0 commit comments

Comments
 (0)