python: update py/weak-cryptographic-algorithm to flag use of ECB block mode

Author: alexrford

python/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql

@@ -13,13 +13,18 @@
 import python
 import semmle.python.Concepts
 
-from Cryptography::CryptographicOperation operation, Cryptography::CryptographicAlgorithm algorithm
+from
+  Cryptography::CryptographicOperation operation, Cryptography::CryptographicAlgorithm algorithm,
+  string msgPrefix
 where
   algorithm = operation.getAlgorithm() and
-  algorithm.isWeak() and
   // `Cryptography::HashingAlgorithm` and `Cryptography::PasswordHashingAlgorithm` are
   // handled by `py/weak-sensitive-data-hashing`
-  algorithm instanceof Cryptography::EncryptionAlgorithm
-select operation,
-  "The cryptographic algorithm " + algorithm.getName() +
-    " is broken or weak, and should not be used."
+  algorithm instanceof Cryptography::EncryptionAlgorithm and
+  (
+    algorithm.isWeak() and
+    msgPrefix = "The cryptographic algorithm " + operation.getAlgorithm().getName()
+  )
+  or
+  operation.getBlockMode().isWeak() and msgPrefix = "The block mode " + operation.getBlockMode()
+select operation, msgPrefix + " is broken or weak, and should not be used."

python/ql/test/query-tests/Security/CWE-327-BrokenCryptoAlgorithm/BrokenCryptoAlgorithm.expected

@@ -1,2 +1,4 @@
 | test_cryptodome.py:11:13:11:42 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 is broken or weak, and should not be used. |
+| test_cryptodome.py:16:13:16:42 | ControlFlowNode for Attribute() | The block mode ECB is broken or weak, and should not be used. |
 | test_cryptography.py:13:13:13:44 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 is broken or weak, and should not be used. |
+| test_cryptography.py:22:13:22:58 | ControlFlowNode for Attribute() | The block mode ECB is broken or weak, and should not be used. |

python/ql/test/query-tests/Security/CWE-327-BrokenCryptoAlgorithm/test_cryptodome.py

@@ -1,5 +1,5 @@
 # snippet from python/ql/test/experimental/library-tests/frameworks/cryptodome/test_rc4.py
-from Cryptodome.Cipher import ARC4
+from Cryptodome.Cipher import ARC4, AES
 
 import os
 
@@ -11,3 +11,8 @@
 encrypted = cipher.encrypt(secret_message) # NOT OK
 
 print(secret_message, encrypted)
+
+cipher = AES.new(key, AES.MODE_ECB)
+encrypted = cipher.encrypt(secret_message) # NOT OK
+
+print(secret_message, encrypted)

python/ql/test/query-tests/Security/CWE-327-BrokenCryptoAlgorithm/test_cryptography.py

@@ -1,5 +1,5 @@
 # snippet from python/ql/test/experimental/library-tests/frameworks/cryptography/test_rc4.py
-from cryptography.hazmat.primitives.ciphers import algorithms, Cipher
+from cryptography.hazmat.primitives.ciphers import algorithms, modes, Cipher
 import os
 
 key = os.urandom(256//8)
@@ -14,3 +14,12 @@
 encrypted += encryptor.finalize()
 
 print(secret_message, encrypted)
+
+algorithm = algorithms.AES(key)
+cipher = Cipher(algorithm, mode=modes.ECB())
+
+encryptor = cipher.encryptor()
+encrypted = encryptor.update(secret_message + b'\x80\x00') # NOT OK
+encrypted += encryptor.finalize()
+
+print(secret_message, encrypted)