Add fixes to python cryptography rules (#3267) (#3272)

* Add fixes to python cryptography rules * Move rule test syntaxt to reflect focus metavariable changes * update mode recommendation to gcn * update fixtest to reflect changes to fix Co-authored-by: Pieter De Cremer (Semgrep) <[email protected]>
semgrep · Jan 12, 2024 · 31e01cd · 31e01cd
1 parent a7d1278
commit 31e01cd
Show file tree

Hide file tree

Showing 15 changed files with 183 additions and 12 deletions.
diff --git a/python/cryptography/security/insecure-cipher-algorithms-arc4.fixed.py b/python/cryptography/security/insecure-cipher-algorithms-arc4.fixed.py
@@ -0,0 +1,17 @@
+# cf. https://github.com/PyCQA/bandit/blob/b78c938c0bd03d201932570f5e054261e10c5750/examples/ciphers.py
+
+from cryptography.hazmat.primitives.ciphers import Cipher
+from cryptography.hazmat.primitives.ciphers import algorithms
+from cryptography.hazmat.primitives.ciphers import modes
+from cryptography.hazmat.backends import default_backend
+from struct import pack
+
+# ruleid:insecure-cipher-algorithm-arc4
+cipher = Cipher(algorithms.AES(key), mode=None, backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message")
+
+# ok:insecure-cipher-algorithm-arc4
+cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message") + encryptor.finalize()
diff --git a/python/cryptography/security/insecure-cipher-algorithms-arc4.yaml b/python/cryptography/security/insecure-cipher-algorithms-arc4.yaml
@@ -1,10 +1,12 @@
 rules:
 - id: insecure-cipher-algorithm-arc4
-  pattern: cryptography.hazmat.primitives.ciphers.algorithms.ARC4(...)
   message: >-
     ARC4 (Alleged RC4) is a stream cipher with serious weaknesses in its initial stream output. 
     Its use is strongly discouraged. ARC4 does not use mode constructions.
     Use a strong symmetric cipher such as EAS instead.
+    With the `cryptography` package it is recommended to use the `Fernet` which is a secure implementation
+    of AES in CBC mode with a 128-bit key. 
+    Alternatively, keep using the `Cipher` class from the hazmat primitives but use the AES algorithm instead.
   metadata:
     source-rule-url: https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/bandit/blacklists/calls.py#L98
     cwe:
@@ -28,3 +30,11 @@ rules:
   severity: WARNING
   languages:
   - python
+  patterns:
+    - pattern: cryptography.hazmat.primitives.ciphers.algorithms.$ARC4($KEY)
+    - pattern-inside: cryptography.hazmat.primitives.ciphers.Cipher(...)
+    - metavariable-regex:
+        metavariable: $ARC4
+        regex: ^(ARC4)$
+    - focus-metavariable: $ARC4
+  fix: AES
diff --git a/python/cryptography/security/insecure-cipher-algorithms-blowfish.fixed.py b/python/cryptography/security/insecure-cipher-algorithms-blowfish.fixed.py
@@ -0,0 +1,18 @@
+# cf. https://github.com/PyCQA/bandit/blob/b78c938c0bd03d201932570f5e054261e10c5750/examples/ciphers.py
+
+from cryptography.hazmat.primitives.ciphers import Cipher
+from cryptography.hazmat.primitives.ciphers import algorithms
+from cryptography.hazmat.primitives.ciphers import modes
+from cryptography.hazmat.backends import default_backend
+from struct import pack
+
+
+# ruleid:insecure-cipher-algorithm-blowfish
+cipher = Cipher(algorithms.AES(key), mode=None, backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message")
+
+# ok:insecure-cipher-algorithm-blowfish
+cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message") + encryptor.finalize()
diff --git a/python/cryptography/security/insecure-cipher-algorithms-blowfish.yaml b/python/cryptography/security/insecure-cipher-algorithms-blowfish.yaml
@@ -1,9 +1,11 @@
 rules:
 - id: insecure-cipher-algorithm-blowfish
-  pattern: cryptography.hazmat.primitives.ciphers.algorithms.Blowfish(...)
   message: >-
     Blowfish is a block cipher developed by Bruce Schneier. It is known to be susceptible to attacks when using weak keys. 
     The author has recommended that users of Blowfish move to newer algorithms such as AES.
+    With the `cryptography` package it is recommended to use `Fernet` which is a secure implementation
+    of AES in CBC mode with a 128-bit key. 
+    Alternatively, keep using the `Cipher` class from the hazmat primitives but use the AES algorithm instead.
   metadata:
     source-rule-url: https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/bandit/blacklists/calls.py#L98
     cwe:
@@ -28,3 +30,10 @@ rules:
   severity: WARNING
   languages:
   - python
+  patterns: 
+    - pattern: cryptography.hazmat.primitives.ciphers.algorithms.$BLOWFISH($KEY)
+    - metavariable-regex:
+        metavariable: $BLOWFISH
+        regex: ^(Blowfish)$
+    - focus-metavariable: $BLOWFISH
+  fix: AES
diff --git a/python/cryptography/security/insecure-cipher-algorithms.fixed.py b/python/cryptography/security/insecure-cipher-algorithms.fixed.py
@@ -0,0 +1,17 @@
+# cf. https://github.com/PyCQA/bandit/blob/b78c938c0bd03d201932570f5e054261e10c5750/examples/ciphers.py
+
+from cryptography.hazmat.primitives.ciphers import Cipher
+from cryptography.hazmat.primitives.ciphers import algorithms
+from cryptography.hazmat.primitives.ciphers import modes
+from cryptography.hazmat.backends import default_backend
+from struct import pack
+
+# ruleid:insecure-cipher-algorithm-idea
+cipher = Cipher(algorithms.AES(key), mode=None, backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message")
+
+# ok:insecure-cipher-algorithm-idea
+cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=default_backend())
+encryptor = cipher.encryptor()
+ct = encryptor.update(b"a secret message") + encryptor.finalize()
diff --git a/python/cryptography/security/insecure-cipher-algorithms.yaml b/python/cryptography/security/insecure-cipher-algorithms.yaml
@@ -1,11 +1,13 @@
 rules:
 - id: insecure-cipher-algorithm-idea
-  pattern: cryptography.hazmat.primitives.ciphers.algorithms.IDEA(...)
   message: >-
     IDEA (International Data Encryption Algorithm) is a block cipher created in 1991. 
     It is an optional component of the OpenPGP standard. This cipher is susceptible to attacks when using weak keys. 
     It is recommended that you do not use this cipher for new applications.
     Use a strong symmetric cipher such as EAS instead.
+    With the `cryptography` package it is recommended to use `Fernet` which is a secure implementation
+    of AES in CBC mode with a 128-bit key. 
+    Alternatively, keep using the `Cipher` class from the hazmat primitives but use the AES algorithm instead.
   metadata:
     source-rule-url: https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/bandit/blacklists/calls.py#L98
     cwe:
@@ -30,3 +32,10 @@ rules:
   severity: WARNING
   languages:
   - python
+  patterns:
+    - pattern: cryptography.hazmat.primitives.ciphers.algorithms.$IDEA($KEY)
+    - metavariable-regex:
+        metavariable: $IDEA
+        regex: ^(IDEA)$
+    - focus-metavariable: $IDEA
+  fix: AES
diff --git a/python/cryptography/security/insecure-cipher-mode-ecb.fixed.py b/python/cryptography/security/insecure-cipher-mode-ecb.fixed.py
@@ -0,0 +1,17 @@
+# cf. https://github.com/PyCQA/bandit/blob/b1411bfb43795d3ffd268bef17a839dee954c2b1/examples/cipher-modes.py
+
+from cryptography.hazmat.primitives.ciphers.modes import CBC
+from cryptography.hazmat.primitives.ciphers.modes import ECB
+
+
+# Insecure mode
+# ruleid: insecure-cipher-mode-ecb
+mode = cryptography.hazmat.primitives.ciphers.modes.GCM(iv)
+
+# Secure cipher and mode
+# ok: insecure-cipher-mode-ecb
+cipher = AES.new(key, blockalgo.MODE_CTR, iv)
+
+# Secure mode
+# ok: insecure-cipher-mode-ecb
+mode = CBC(iv)
diff --git a/python/cryptography/security/insecure-cipher-mode-ecb.yaml b/python/cryptography/security/insecure-cipher-mode-ecb.yaml
@@ -1,11 +1,10 @@
 rules:
 - id: insecure-cipher-mode-ecb
-  pattern: cryptography.hazmat.primitives.ciphers.modes.ECB(...)
   message: >-
     ECB (Electronic Code Book) is the simplest mode of operation for block ciphers. 
     Each block of data is encrypted in the same way. 
     This means identical plaintext blocks will always result in identical ciphertext blocks, which can leave significant patterns in the output.
-    Use a different, more secure mode instead.
+    Use a different, cryptographically strong mode instead, such as GCM.
   metadata:
     source-rule-url: https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/bandit/blacklists/calls.py#L101
     cwe:
@@ -30,3 +29,5 @@ rules:
   severity: WARNING
   languages:
   - python
+  pattern: cryptography.hazmat.primitives.ciphers.modes.ECB($IV)
+  fix: cryptography.hazmat.primitives.ciphers.modes.GCM($IV)
diff --git a/python/cryptography/security/insecure-hash-algorithms-md5.fixed.py b/python/cryptography/security/insecure-hash-algorithms-md5.fixed.py
@@ -0,0 +1,10 @@
+# cf. https://github.com/PyCQA/bandit/blob/b78c938c0bd03d201932570f5e054261e10c5750/examples/crypto-md5.py
+
+from cryptography.hazmat.primitives import hashes
+
+# ruleid:insecure-hash-algorithm-md5
+hashes.SHA256()
+# ok:insecure-hash-algorithm-md5
+hashes.SHA256()
+# ok:insecure-hash-algorithm-md5
+hashes.SHA3_256()
diff --git a/python/cryptography/security/insecure-hash-algorithms-md5.yaml b/python/cryptography/security/insecure-hash-algorithms-md5.yaml
@@ -1,6 +1,5 @@
 rules:
 - id: insecure-hash-algorithm-md5
-  pattern: cryptography.hazmat.primitives.hashes.MD5(...)
   message: >-
     Detected MD5 hash algorithm which is considered insecure. MD5 is not
     collision resistant and is therefore not suitable as a cryptographic
@@ -32,3 +31,10 @@ rules:
   severity: WARNING
   languages:
   - python
+  patterns:
+    - pattern: cryptography.hazmat.primitives.hashes.$MD5()
+    - metavariable-regex:
+        metavariable: $MD5
+        regex: ^(MD5)$
+    - focus-metavariable: $MD5
+  fix: SHA256
diff --git a/python/cryptography/security/insufficient-dsa-key-size.fixed.py b/python/cryptography/security/insufficient-dsa-key-size.fixed.py
@@ -0,0 +1,18 @@
+from cryptography.hazmat import backends
+from cryptography.hazmat.primitives.asymmetric import dsa
+
+# ok: insufficient-dsa-key-size
+dsa.generate_private_key(key_size=2048,
+                         backend=backends.default_backend())
+
+# ok: insufficient-dsa-key-size
+dsa.generate_private_key(2048,
+                         backend=backends.default_backend())
+
+# ruleid: insufficient-dsa-key-size
+dsa.generate_private_key(key_size=2048,
+                         backend=backends.default_backend())
+
+# ruleid: insufficient-dsa-key-size
+dsa.generate_private_key(2048,
+                         backend=backends.default_backend())
diff --git a/python/cryptography/security/insufficient-dsa-key-size.yaml b/python/cryptography/security/insufficient-dsa-key-size.yaml
@@ -8,6 +8,9 @@ rules:
   - metavariable-comparison:
       metavariable: $SIZE
       comparison: $SIZE < 2048
+  - focus-metavariable: $SIZE
+  fix: |
+      2048
   message: >-
     Detected an insufficient key size for DSA. NIST recommends
     a key size of 2048 or higher.

diff --git a/python/cryptography/security/insufficient-rsa-key-size.fixed.py b/python/cryptography/security/insufficient-rsa-key-size.fixed.py
@@ -0,0 +1,33 @@
+import os
+from cryptography.hazmat import backends
+from cryptography.hazmat.primitives.asymmetric import rsa
+
+rsa.generate_private_key(public_exponent=65537,
+# ok: insufficient-rsa-key-size
+                         key_size=2048,
+                         backend=backends.default_backend())
+
+rsa.generate_private_key(65537,
+# ok: insufficient-rsa-key-size
+                         2048,
+                         backends.default_backend())
+
+rsa.generate_private_key(public_exponent=65537,
+# ok: insufficient-rsa-key-size
+                         key_size=os.getenv("KEY_SIZE"),
+                         backend=backends.default_backend())
+
+rsa.generate_private_key(65537,
+# ok: insufficient-rsa-key-size
+                         2048,
+                         backends.default_backend())
+
+rsa.generate_private_key(public_exponent=65537,
+# ruleid: insufficient-rsa-key-size
+                         key_size=2048,
+                         backend=backends.default_backend())
+
+rsa.generate_private_key(65537,
+# ruleid: insufficient-rsa-key-size
+                         2048,
+                         backends.default_backend())
diff --git a/python/cryptography/security/insufficient-rsa-key-size.py b/python/cryptography/security/insufficient-rsa-key-size.py
@@ -2,32 +2,32 @@
 from cryptography.hazmat import backends
 from cryptography.hazmat.primitives.asymmetric import rsa
 
-# ok: insufficient-rsa-key-size
 rsa.generate_private_key(public_exponent=65537,
+# ok: insufficient-rsa-key-size
                          key_size=2048,
                          backend=backends.default_backend())
 
-# ok: insufficient-rsa-key-size
 rsa.generate_private_key(65537,
+# ok: insufficient-rsa-key-size
                          2048,
                          backends.default_backend())
 
-# ok: insufficient-rsa-key-size
 rsa.generate_private_key(public_exponent=65537,
+# ok: insufficient-rsa-key-size
                          key_size=os.getenv("KEY_SIZE"),
                          backend=backends.default_backend())
 
-# ok: insufficient-rsa-key-size
 rsa.generate_private_key(65537,
+# ok: insufficient-rsa-key-size
                          2048,
                          backends.default_backend())
 
-# ruleid: insufficient-rsa-key-size
 rsa.generate_private_key(public_exponent=65537,
+# ruleid: insufficient-rsa-key-size
                          key_size=1024,
                          backend=backends.default_backend())
 
-# ruleid: insufficient-rsa-key-size
 rsa.generate_private_key(65537,
+# ruleid: insufficient-rsa-key-size
                          1024,
                          backends.default_backend())
diff --git a/python/cryptography/security/insufficient-rsa-key-size.yaml b/python/cryptography/security/insufficient-rsa-key-size.yaml
@@ -8,6 +8,9 @@ rules:
   - metavariable-comparison:
       metavariable: $SIZE
       comparison: $SIZE < 2048
+  - focus-metavariable: $SIZE
+  fix : |
+      2048
   message: >-
     Detected an insufficient key size for RSA. NIST recommends
     a key size of 2048 or higher.