From 0b2716918345b2bff0bb5c36086d5de368125536 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Tue, 24 Jul 2018 03:53:39 -0700 Subject: [PATCH] bpo-34164: Fix handling of incorrect padding in base64.b32decode(). (GH-8351) (GH-8435) Now base64.Error is always raised instead of UnboundLocalError or OverflowError. (cherry picked from commit ac0b3c2f4d86fc056b833a4e6b9a380741244a63) Co-authored-by: Serhiy Storchaka --- Lib/base64.py | 17 +++++------------ Lib/test/test_base64.py | 19 ++++++++++++++----- .../2018-07-20-18-06-00.bpo-34164.fNfT-q.rst | 2 ++ 3 files changed, 21 insertions(+), 17 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-07-20-18-06-00.bpo-34164.fNfT-q.rst diff --git a/Lib/base64.py b/Lib/base64.py index eb8f258a2d1..2be9c395a96 100755 --- a/Lib/base64.py +++ b/Lib/base64.py @@ -231,23 +231,16 @@ def b32decode(s, casefold=False, map01=None): raise binascii.Error('Non-base32 digit found') from None decoded += acc.to_bytes(5, 'big') # Process the last, partial quanta - if padchars: + if l % 8 or padchars not in {0, 1, 3, 4, 6}: + raise binascii.Error('Incorrect padding') + if padchars and decoded: acc <<= 5 * padchars last = acc.to_bytes(5, 'big') - if padchars == 1: - decoded[-5:] = last[:-1] - elif padchars == 3: - decoded[-5:] = last[:-2] - elif padchars == 4: - decoded[-5:] = last[:-3] - elif padchars == 6: - decoded[-5:] = last[:-4] - else: - raise binascii.Error('Incorrect padding') + leftover = (43 - 5 * padchars) // 8 # 1: 4, 3: 3, 4: 2, 6: 1 + decoded[-5:] = last[:leftover] return bytes(decoded) - # RFC 3548, Base 16 Alphabet specifies uppercase, but hexlify() returns # lowercase. The RFC also recommends against accepting input case # insensitively. diff --git a/Lib/test/test_base64.py b/Lib/test/test_base64.py index 47547396b8c..2a4cc2acad2 100644 --- a/Lib/test/test_base64.py +++ b/Lib/test/test_base64.py @@ -343,11 +343,20 @@ class BaseXYTestCase(unittest.TestCase): self.assertRaises(binascii.Error, base64.b32decode, data_str) def test_b32decode_error(self): - for data in [b'abc', b'ABCDEF==', b'==ABCDEF']: - with self.assertRaises(binascii.Error): - base64.b32decode(data) - with self.assertRaises(binascii.Error): - base64.b32decode(data.decode('ascii')) + tests = [b'abc', b'ABCDEF==', b'==ABCDEF'] + prefixes = [b'M', b'ME', b'MFRA', b'MFRGG', b'MFRGGZA', b'MFRGGZDF'] + for i in range(0, 17): + if i: + tests.append(b'='*i) + for prefix in prefixes: + if len(prefix) + i != 8: + tests.append(prefix + b'='*i) + for data in tests: + with self.subTest(data=data): + with self.assertRaises(binascii.Error): + base64.b32decode(data) + with self.assertRaises(binascii.Error): + base64.b32decode(data.decode('ascii')) def test_b16encode(self): eq = self.assertEqual diff --git a/Misc/NEWS.d/next/Library/2018-07-20-18-06-00.bpo-34164.fNfT-q.rst b/Misc/NEWS.d/next/Library/2018-07-20-18-06-00.bpo-34164.fNfT-q.rst new file mode 100644 index 00000000000..99bf169308c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-07-20-18-06-00.bpo-34164.fNfT-q.rst @@ -0,0 +1,2 @@ +:func:`base64.b32decode` could raise UnboundLocalError or OverflowError for +incorrect padding. Now it always raises :exc:`base64.Error` in these cases.