Merge pull request #10536 from karimhamdanali/ecbmode

Swift: check for using ECB encryption mode

Author: MathiasVP

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

@@ -0,0 +1,22 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>ECB should not be used as a mode for encryption as it has dangerous weaknesses. Data is encrypted the same way every time, which means that the same plaintext input will always produce the same ciphertext. This behavior makes messages encrypted with ECB
+ more vulnerable to replay attacks.</p>
+  </overview>
+
+  <recommendation>
+    <p>Use a different cipher mode such as CBC.</p>
+  </recommendation>
+
+  <example>
+    <p>The following example shows six cases of instantiating a cipher with various encryption keys and block modes. In the 'BAD' cases, the mode of encryption is ECB, making the encrypted data vulnerable to replay attacks.  In the 'GOOD' cases, the encryption mode is CBC, which protects the encrypted data against replay attacks.</p>
+    <sample src="ECBEncryption.swift" />
+  </example>
+
+  <references>
+    <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>
+  </references>
+</qhelp>

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

@@ -0,0 +1,80 @@
+/**
+ * @name Encryption using ECB
+ * @description Using the ECB encryption mode makes code vulnerable to replay attacks.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id swift/ecb-encryption
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+/**
+ * An `Expr` that is used to initialize the block mode of a cipher.
+ */
+abstract class BlockMode extends Expr { }
+
+/**
+ * An `Expr` that is used to form an `AES` cipher.
+ */
+class AES extends BlockMode {
+  AES() {
+    // `blockMode` arg in `AES.init` is a sink
+    exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
+      c.getName() = "AES" and
+      c.getAMember() = f and
+      f.getName() = ["init(key:blockMode:)", "init(key:blockMode:padding:)"] and
+      call.getStaticTarget() = f and
+      call.getArgument(1).getExpr() = this
+    )
+  }
+}
+
+/**
+ * An `Expr` that is used to form a `Blowfish` cipher.
+ */
+class Blowfish extends BlockMode {
+  Blowfish() {
+    // `blockMode` arg in `Blowfish.init` is a sink
+    exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
+      c.getName() = "Blowfish" and
+      c.getAMember() = f and
+      f.getName() = "init(key:blockMode:padding:)" and
+      call.getStaticTarget() = f and
+      call.getArgument(1).getExpr() = this
+    )
+  }
+}
+
+/**
+ * A taint configuration from the constructor of ECB mode to expressions that use
+ * it to initialize a cipher.
+ */
+class EcbEncryptionConfig extends DataFlow::Configuration {
+  EcbEncryptionConfig() { this = "EcbEncryptionConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    exists(StructDecl s, AbstractFunctionDecl f, CallExpr call |
+      s.getName() = "ECB" and
+      s.getAMember() = f and
+      f.getName() = "init()" and
+      call.getStaticTarget() = f and
+      node.asExpr() = call
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof BlockMode }
+}
+
+// The query itself
+from EcbEncryptionConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
+where config.hasFlowPath(sourceNode, sinkNode)
+select sinkNode.getNode(), sourceNode, sinkNode,
+  "The initialization of the cipher '" + sinkNode.getNode().toString() +
+    "' uses the insecure ECB block mode from $@.", sourceNode, sourceNode.getNode().toString()

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

@@ -0,0 +1,18 @@
+
+func encrypt(key : Key, padding : Padding) {
+	// ...
+
+	// BAD: ECB is used for block mode
+	let blockMode = ECB()
+	_ = try AES(key: key, blockMode: blockMode, padding: padding)
+	_ = try AES(key: key, blockMode: blockMode)
+	_ = try Blowfish(key: key, blockMode: blockMode, padding: padding)
+
+	// GOOD: ECB is not used for block mode
+	let blockMode = CBC()
+	_ = try AES(key: key, blockMode: blockMode, padding: padding)
+	_ = try AES(key: key, blockMode: blockMode)
+	_ = try Blowfish(key: key, blockMode: blockMode, padding: padding)
+
+	// ...
+}

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

@@ -0,0 +1,30 @@
+edges
+| test.swift:34:9:34:13 | call to init() :  | test.swift:54:37:54:53 | call to getECBBlockMode() |
+| test.swift:34:9:34:13 | call to init() :  | test.swift:55:37:55:53 | call to getECBBlockMode() |
+| test.swift:34:9:34:13 | call to init() :  | test.swift:67:42:67:58 | call to getECBBlockMode() |
+| test.swift:45:12:45:16 | call to init() :  | test.swift:50:37:50:37 | ecb |
+| test.swift:45:12:45:16 | call to init() :  | test.swift:51:37:51:37 | ecb |
+| test.swift:45:12:45:16 | call to init() :  | test.swift:65:42:65:42 | ecb |
+nodes
+| test.swift:34:9:34:13 | call to init() :  | semmle.label | call to init() :  |
+| test.swift:45:12:45:16 | call to init() :  | semmle.label | call to init() :  |
+| test.swift:50:37:50:37 | ecb | semmle.label | ecb |
+| test.swift:51:37:51:37 | ecb | semmle.label | ecb |
+| test.swift:52:37:52:41 | call to init() | semmle.label | call to init() |
+| test.swift:53:37:53:41 | call to init() | semmle.label | call to init() |
+| test.swift:54:37:54:53 | call to getECBBlockMode() | semmle.label | call to getECBBlockMode() |
+| test.swift:55:37:55:53 | call to getECBBlockMode() | semmle.label | call to getECBBlockMode() |
+| test.swift:65:42:65:42 | ecb | semmle.label | ecb |
+| test.swift:66:42:66:46 | call to init() | semmle.label | call to init() |
+| test.swift:67:42:67:58 | call to getECBBlockMode() | semmle.label | call to getECBBlockMode() |
+subpaths
+#select
+| test.swift:50:37:50:37 | ecb | test.swift:45:12:45:16 | call to init() :  | test.swift:50:37:50:37 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@. | test.swift:45:12:45:16 | call to init() :  | call to init() |
+| test.swift:51:37:51:37 | ecb | test.swift:45:12:45:16 | call to init() :  | test.swift:51:37:51:37 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@. | test.swift:45:12:45:16 | call to init() :  | call to init() |
+| test.swift:52:37:52:41 | call to init() | test.swift:52:37:52:41 | call to init() | test.swift:52:37:52:41 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@. | test.swift:52:37:52:41 | call to init() | call to init() |
+| test.swift:53:37:53:41 | call to init() | test.swift:53:37:53:41 | call to init() | test.swift:53:37:53:41 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@. | test.swift:53:37:53:41 | call to init() | call to init() |
+| test.swift:54:37:54:53 | call to getECBBlockMode() | test.swift:34:9:34:13 | call to init() :  | test.swift:54:37:54:53 | call to getECBBlockMode() | The initialization of the cipher 'call to getECBBlockMode()' uses the insecure ECB block mode from $@. | test.swift:34:9:34:13 | call to init() :  | call to init() |
+| test.swift:55:37:55:53 | call to getECBBlockMode() | test.swift:34:9:34:13 | call to init() :  | test.swift:55:37:55:53 | call to getECBBlockMode() | The initialization of the cipher 'call to getECBBlockMode()' uses the insecure ECB block mode from $@. | test.swift:34:9:34:13 | call to init() :  | call to init() |
+| test.swift:65:42:65:42 | ecb | test.swift:45:12:45:16 | call to init() :  | test.swift:65:42:65:42 | ecb | The initialization of the cipher 'ecb' uses the insecure ECB block mode from $@. | test.swift:45:12:45:16 | call to init() :  | call to init() |
+| test.swift:66:42:66:46 | call to init() | test.swift:66:42:66:46 | call to init() | test.swift:66:42:66:46 | call to init() | The initialization of the cipher 'call to init()' uses the insecure ECB block mode from $@. | test.swift:66:42:66:46 | call to init() | call to init() |
+| test.swift:67:42:67:58 | call to getECBBlockMode() | test.swift:34:9:34:13 | call to init() :  | test.swift:67:42:67:58 | call to getECBBlockMode() | The initialization of the cipher 'call to getECBBlockMode()' uses the insecure ECB block mode from $@. | test.swift:34:9:34:13 | call to init() :  | call to init() |

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

@@ -0,0 +1 @@
+queries/Security/ECB-Encryption/ECBEncryption.ql

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

@@ -0,0 +1,72 @@
+
+// --- stubs ---
+
+// These stubs roughly follows the same structure as classes from CryptoSwift
+class AES
+{
+	init(key: Array<UInt8>, blockMode: BlockMode, padding: Padding) { }
+	init(key: Array<UInt8>, blockMode: BlockMode) { }
+}
+
+class Blowfish
+{
+	init(key: Array<UInt8>, blockMode: BlockMode, padding: Padding) { }
+}
+
+protocol BlockMode { }
+
+struct ECB: BlockMode { 
+	init() { }
+}
+
+struct CBC: BlockMode { 
+	init() { }
+}
+
+protocol PaddingProtocol { }
+
+enum Padding: PaddingProtocol {
+  case noPadding, zeroPadding, pkcs7, pkcs5, eme_pkcs1v15, emsa_pkcs1v15, iso78164, iso10126
+}
+
+// Create some inter-procedural dependencies
+func getECBBlockMode() -> BlockMode {
+	return ECB()
+}
+
+func getCBCBlockMode() ->  BlockMode {
+	return CBC()
+}
+
+// --- tests ---
+
+func test1() {
+	let key: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05, 0xaf, 0x46, 0x58, 0x2d, 0x66, 0x52, 0x10, 0xae, 0x86, 0xd3, 0x8e, 0x8f]
+	let ecb = ECB()
+	let cbc = CBC()
+	let padding = Padding.noPadding
+
+	// AES test cases
+	let ab1 = AES(key: key, blockMode: ecb, padding: padding) // BAD
+	let ab2 = AES(key: key, blockMode: ecb) // BAD
+	let ab3 = AES(key: key, blockMode: ECB(), padding: padding) // BAD
+	let ab4 = AES(key: key, blockMode: ECB()) // BAD
+	let ab5 = AES(key: key, blockMode: getECBBlockMode(), padding: padding) // BAD
+	let ab6 = AES(key: key, blockMode: getECBBlockMode()) // BAD
+
+	let ag1 = AES(key: key, blockMode: cbc, padding: padding) // GOOD
+	let ag2 = AES(key: key, blockMode: cbc) // GOOD
+	let ag3 = AES(key: key, blockMode: CBC(), padding: padding) // GOOD
+	let ag4 = AES(key: key, blockMode: CBC()) // GOOD
+	let ag5 = AES(key: key, blockMode: getCBCBlockMode(), padding: padding) // GOOD
+	let ag6 = AES(key: key, blockMode: getCBCBlockMode()) // GOOD
+
+	// Blowfish test cases
+	let bb1 = Blowfish(key: key, blockMode: ecb, padding: padding) // BAD
+	let bb2 = Blowfish(key: key, blockMode: ECB(), padding: padding) // BAD
+	let bb3 = Blowfish(key: key, blockMode: getECBBlockMode(), padding: padding) // BAD
+
+	let bg1 = Blowfish(key: key, blockMode: cbc, padding: padding) // GOOD
+	let bg2 = Blowfish(key: key, blockMode: CBC(), padding: padding) // GOOD
+	let bg3 = Blowfish(key: key, blockMode: getCBCBlockMode(), padding: padding) // GOOD
+}