From f878c374086e672e7806fdd18401ec6b71cfa960 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Sun, 15 Nov 2020 15:48:27 +0100
Subject: [PATCH] Fix #164: Add padding length check as described by PKCS#1
 v1.5

According to PKCS#1 v1.5, the padding should be at least 8 bytes long.
See https://tools.ietf.org/html/rfc8017#section-7.2.2 step 3 for more info.
---
 CHANGELOG.md        |  2 ++
 rsa/pkcs1.py        |  7 ++++++-
 tests/test_pkcs1.py | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1838377..77ad5cd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 - Fix #165: CVE-2020-25658 - Bleichenbacher-style timing oracle in PKCS#1 v1.5
   decryption code
+- Add padding length check as described by PKCS#1 v1.5 (Fixes
+  [#164](https://github.com/sybrenstuvel/python-rsa/issues/164))
 
 
 ## Version 4.4 & 4.6 - released 2020-06-12
diff --git a/rsa/pkcs1.py b/rsa/pkcs1.py
index 19e24c7..8a85b1c 100644
--- a/rsa/pkcs1.py
+++ b/rsa/pkcs1.py
@@ -262,7 +262,12 @@ def decrypt(crypto: bytes, priv_key: key.PrivateKey) -> bytes:
         sep_idx = cleartext.index(b'\x00', 2)
     except ValueError:
         sep_idx = -1
-    sep_idx_bad = sep_idx < 0
+    # sep_idx indicates the position of the `\x00` separator that separates the
+    # padding from the actual message. The padding should be at least 8 bytes
+    # long (see https://tools.ietf.org/html/rfc8017#section-7.2.2 step 3), which
+    # means the separator should be at least at index 10 (because of the
+    # `\x00\x02` marker that preceeds it).
+    sep_idx_bad = sep_idx < 10
 
     anything_bad = crypto_len_bad | cleartext_marker_bad | sep_idx_bad
     if anything_bad:
diff --git a/tests/test_pkcs1.py b/tests/test_pkcs1.py
index f7baf7f..64fb0c5 100644
--- a/tests/test_pkcs1.py
+++ b/tests/test_pkcs1.py
@@ -183,3 +183,36 @@ def test_apppend_zeroes(self):
         signature = signature + bytes.fromhex('0000')
         with self.assertRaises(rsa.VerificationError):
             pkcs1.verify(message, signature, self.pub)
+
+
+class PaddingSizeTest(unittest.TestCase):
+    def test_too_little_padding(self):
+        """Padding less than 8 bytes should be rejected."""
+
+        # Construct key that will be small enough to need only 7 bytes of padding.
+        # This key is 168 bit long, and was generated with rsa.newkeys(nbits=168).
+        self.private_key = rsa.PrivateKey.load_pkcs1(b'''
+-----BEGIN RSA PRIVATE KEY-----
+MHkCAQACFgCIGbbNSkIRLtprxka9NgOf5UxgxCMCAwEAAQIVQqymO0gHubdEVS68
+CdCiWmOJxVfRAgwBQM+e1JJwMKmxSF0CCmya6CFxO8Evdn8CDACMM3AlVC4FhlN8
+3QIKC9cjoam/swMirwIMAR7Br9tdouoH7jAE
+-----END RSA PRIVATE KEY-----
+        ''')
+        self.public_key = rsa.PublicKey(n=self.private_key.n, e=self.private_key.e)
+
+        cyphertext = self.encrypt_with_short_padding(b'op je hoofd')
+        with self.assertRaises(rsa.DecryptionError):
+            rsa.decrypt(cyphertext, self.private_key)
+
+    def encrypt_with_short_padding(self, message: bytes) -> bytes:
+        # This is a copy of rsa.pkcs1.encrypt() adjusted to use the wrong padding length.
+        keylength = rsa.common.byte_size(self.public_key.n)
+
+        # The word 'padding' has 7 letters, so is one byte short of a valid padding length.
+        padded = b'\x00\x02padding\x00' + message
+
+        payload = rsa.transform.bytes2int(padded)
+        encrypted_value = rsa.core.encrypt_int(payload, self.public_key.e, self.public_key.n)
+        cyphertext = rsa.transform.int2bytes(encrypted_value, keylength)
+
+        return cyphertext