From b96e02e320cc8b38907b0cbba0e0de8f48d14c5b Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Fri, 27 Sep 2019 17:50:10 +0200 Subject: [PATCH] ensure that the encoding is actually the minimal one for length and integer --- src/ecdsa/der.py | 17 ++++++++++-- src/ecdsa/test_der.py | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/ecdsa/test_der.py diff --git a/src/ecdsa/der.py b/src/ecdsa/der.py index fec550d5..3ed7f6c3 100644 --- a/src/ecdsa/der.py +++ b/src/ecdsa/der.py @@ -137,6 +137,15 @@ def remove_integer(string): else ord(numberbytes[0]) if not msb < 0x80: raise UnexpectedDER("Negative integers are not supported") + # check if the encoding is the minimal one (DER requirement) + if length > 1 and not msb: + # leading zero byte is allowed if the integer would have been + # considered a negative number otherwise + smsb = numberbytes[1] if isinstance(numberbytes[1], integer_types) \ + else ord(numberbytes[1]) + if smsb < 0x80: + raise UnexpectedDER("Invalid encoding of integer, unnecessary " + "zero padding bytes") return int(binascii.hexlify(numberbytes), 16), rest @@ -179,9 +188,13 @@ def read_length(string): # big-endian llen = num & 0x7f if not llen: - raise UnexpectedDER("Invalid length encoding, length byte is 0") + raise UnexpectedDER("Invalid length encoding, length of length is 0") if llen > len(string)-1: - raise UnexpectedDER("ran out of length bytes") + raise UnexpectedDER("Length of length longer than provided buffer") + # verify that the encoding is minimal possible (DER requirement) + msb = string[1] if isinstance(string[1], integer_types) else ord(string[1]) + if not msb or llen == 1 and msb < 0x80: + raise UnexpectedDER("Not minimal encoding of length") return int(binascii.hexlify(string[1:1+llen]), 16), 1+llen diff --git a/src/ecdsa/test_der.py b/src/ecdsa/test_der.py new file mode 100644 index 00000000..fa79b934 --- /dev/null +++ b/src/ecdsa/test_der.py @@ -0,0 +1,63 @@ + +import unittest +from .der import remove_integer, UnexpectedDER, read_length +from six import b + +class TestRemoveInteger(unittest.TestCase): + # DER requires the integers to be 0-padded only if they would be + # interpreted as negative, check if those errors are detected + def test_non_minimal_encoding(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x02\x00\x01')) + + def test_negative_with_high_bit_set(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x01\x80')) + + def test_minimal_with_high_bit_set(self): + val, rem = remove_integer(b('\x02\x02\x00\x80')) + + self.assertEqual(val, 0x80) + self.assertFalse(rem) + + def test_two_zero_bytes_with_high_bit_set(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x03\x00\x00\xff')) + + def test_zero_length_integer(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x00')) + + def test_empty_string(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('')) + + +class TestReadLength(unittest.TestCase): + # DER requires the lengths between 0 and 127 to be encoded using the short + # form and lengths above that encoded with minimal number of bytes + # necessary + def test_zero_length(self): + self.assertEqual((0, 1), read_length(b('\x00'))) + + def test_two_byte_zero_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x81\x00')) + + def test_two_byte_small_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x81\x7f')) + + def test_long_form_with_zero_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x80')) + + def test_smallest_two_byte_length(self): + self.assertEqual((128, 2), read_length(b('\x81\x80'))) + + def test_zero_padded_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x82\x00\x80')) + + def test_two_three_byte_length(self): + self.assertEqual((256, 3), read_length(b'\x82\x01\x00'))