Robust signature decoding

.travis.yml

@@ -1,3 +1,6 @@
+# workaround for 3.7 not available in default configuration
+# travis-ci/travis-ci#9815
+dist: trusty
 sudo: false
 language: python
 cache: pip

build-requirements-2.6.txt

@@ -1,3 +1,4 @@
 tox
 coveralls<1.3.0
 idna<2.8
+unittest2

src/ecdsa/der.py

@@ -65,8 +65,8 @@ def encode_number(n):
 def remove_constructed(string):
     s0 = string[0] if isinstance(string[0], integer_types) else ord(string[0])
     if (s0 & 0xe0) != 0xa0:
-        raise UnexpectedDER("wanted constructed tag (0xa0-0xbf), got 0x%02x"
-                            % s0)
+        raise UnexpectedDER("wanted type 'constructed tag' (0xa0-0xbf), "
+                            "got 0x%02x" % s0)
     tag = s0 & 0x1f
     length, llen = read_length(string[1:])
     body = string[1+llen:1+llen+length]
@@ -75,18 +75,23 @@ def remove_constructed(string):
 
 
 def remove_sequence(string):
+    if not string:
+        raise UnexpectedDER("Empty string does not encode a sequence")
     if not string.startswith(b("\x30")):
-        n = string[0] if isinstance(string[0], integer_types) else ord(string[0])
-        raise UnexpectedDER("wanted sequence (0x30), got 0x%02x" % n)
+        n = string[0] if isinstance(string[0], integer_types) else \
+                ord(string[0])
+        raise UnexpectedDER("wanted type 'sequence' (0x30), got 0x%02x" % n)
     length, lengthlength = read_length(string[1:])
+    if length > len(string) - 1 - lengthlength:
+        raise UnexpectedDER("Length longer than the provided buffer")
     endseq = 1+lengthlength+length
     return string[1+lengthlength:endseq], string[endseq:]
 
 
 def remove_octet_string(string):
     if not string.startswith(b("\x04")):
         n = string[0] if isinstance(string[0], integer_types) else ord(string[0])
-        raise UnexpectedDER("wanted octetstring (0x04), got 0x%02x" % n)
+        raise UnexpectedDER("wanted type 'octetstring' (0x04), got 0x%02x" % n)
     length, llen = read_length(string[1:])
     body = string[1+llen:1+llen+length]
     rest = string[1+llen+length:]
@@ -96,7 +101,7 @@ def remove_octet_string(string):
 def remove_object(string):
     if not string.startswith(b("\x06")):
         n = string[0] if isinstance(string[0], integer_types) else ord(string[0])
-        raise UnexpectedDER("wanted object (0x06), got 0x%02x" % n)
+        raise UnexpectedDER("wanted type 'object' (0x06), got 0x%02x" % n)
     length, lengthlength = read_length(string[1:])
     body = string[1+lengthlength:1+lengthlength+length]
     rest = string[1+lengthlength+length:]
@@ -114,14 +119,33 @@ def remove_object(string):
 
 
 def remove_integer(string):
+    if not string:
+        raise UnexpectedDER("Empty string is an invalid encoding of an "
+                            "integer")
     if not string.startswith(b("\x02")):
-        n = string[0] if isinstance(string[0], integer_types) else ord(string[0])
-        raise UnexpectedDER("wanted integer (0x02), got 0x%02x" % n)
+        n = string[0] if isinstance(string[0], integer_types) \
+                else ord(string[0])
+        raise UnexpectedDER("wanted type 'integer' (0x02), got 0x%02x" % n)
     length, llen = read_length(string[1:])
+    if length > len(string) - 1 - llen:
+        raise UnexpectedDER("Length longer than provided buffer")
+    if length == 0:
+        raise UnexpectedDER("0-byte long encoding of integer")
     numberbytes = string[1+llen:1+llen+length]
     rest = string[1+llen+length:]
-    nbytes = numberbytes[0] if isinstance(numberbytes[0], integer_types) else ord(numberbytes[0])
-    assert nbytes < 0x80  # can't support negative numbers yet
+    msb = numberbytes[0] if isinstance(numberbytes[0], integer_types) \
+            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
 
 
@@ -154,15 +178,23 @@ def encode_length(l):
 
 
 def read_length(string):
+    if not string:
+        raise UnexpectedDER("Empty string can't encode valid length value")
     num = string[0] if isinstance(string[0], integer_types) else ord(string[0])
     if not (num & 0x80):
         # short form
         return (num & 0x7f), 1
     # else long-form: b0&0x7f is number of additional base256 length bytes,
     # big-endian
     llen = num & 0x7f
+    if not llen:
+        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
 
 

src/ecdsa/keys.py

@@ -7,7 +7,7 @@
 from .ecdsa import RSZeroError
 from .util import string_to_number, number_to_string, randrange
 from .util import sigencode_string, sigdecode_string
-from .util import oid_ecPublicKey, encoded_oid_ecPublicKey
+from .util import oid_ecPublicKey, encoded_oid_ecPublicKey, MalformedSignature
 from six import PY3, b
 from hashlib import sha1
 
@@ -136,11 +136,14 @@ def verify_digest(self, signature, digest, sigdecode=sigdecode_string):
                                  "for your digest (%d)" % (self.curve.name,
                                                            8 * len(digest)))
         number = string_to_number(digest)
-        r, s = sigdecode(signature, self.pubkey.order)
+        try:
+            r, s = sigdecode(signature, self.pubkey.order)
+        except (der.UnexpectedDER, MalformedSignature) as e:
+            raise BadSignatureError("Malformed formatting of signature", e)
         sig = ecdsa.Signature(r, s)
         if self.pubkey.verifies(number, sig):
             return True
-        raise BadSignatureError
+        raise BadSignatureError("Signature verification failed")
 
 
 class SigningKey:

src/ecdsa/test_der.py

@@ -0,0 +1,94 @@
+
+# compatibility with Python 2.6, for that we need unittest2 package,
+# which is not available on 3.3 or 3.4
+try:
+    import unittest2 as unittest
+except ImportError:
+    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(''))
+
+    def test_encoding_of_zero(self):
+        val, rem = remove_integer(b('\x02\x01\x00'))
+
+        self.assertEqual(val, 0)
+        self.assertFalse(rem)
+
+    def test_encoding_of_127(self):
+        val, rem = remove_integer(b('\x02\x01\x7f'))
+
+        self.assertEqual(val, 127)
+        self.assertFalse(rem)
+
+    def test_encoding_of_128(self):
+        val, rem = remove_integer(b('\x02\x02\x00\x80'))
+
+        self.assertEqual(val, 128)
+        self.assertFalse(rem)
+
+
+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'))
+
+    def test_empty_string(self):
+        with self.assertRaises(UnexpectedDER):
+            read_length(b(''))
+
+    def test_length_overflow(self):
+        with self.assertRaises(UnexpectedDER):
+            read_length(b('\x83\x01\x00'))

src/ecdsa/test_malformed_sigs.py

@@ -0,0 +1,87 @@
+from __future__ import with_statement, division
+
+import pytest
+import hashlib
+
+from six import b, binary_type
+from .keys import SigningKey, VerifyingKey
+from .keys import BadSignatureError
+from .util import sigencode_der, sigencode_string
+from .util import sigdecode_der, sigdecode_string
+from .curves import curves, NIST256p, NIST521p
+
+der_sigs = []
+example_data = b("some data to sign")
+
+# Just NIST256p with SHA256 is 560 test cases, all curves with all hashes is
+# few thousand slow test cases; execute the most interesting only
+
+#for curve in curves:
+for curve in [NIST521p]:
+    #for hash_alg in ["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]:
+    for hash_alg in ["sha256"]:
+        key = SigningKey.generate(curve)
+        signature = key.sign(example_data, hashfunc=getattr(hashlib, hash_alg),
+                             sigencode=sigencode_der)
+        for pos in range(len(signature)):
+            for xor in (1<<i for i in range(8)):
+                der_sigs.append(pytest.param(
+                    key.verifying_key, hash_alg,
+                    signature, pos, xor,
+                    id="{0}-{1}-pos-{2}-xor-{3}".format(
+                        curve.name, hash_alg, pos, xor)))
+
+
+@pytest.mark.parametrize("verifying_key,hash_alg,signature,pos,xor", der_sigs)
+def test_fuzzed_der_signatures(verifying_key, hash_alg, signature, pos, xor):
+    # check if a malformed DER encoded signature causes the same exception
+    # to be raised irrespective of the type of error
+    sig = bytearray(signature)
+    sig[pos] ^= xor
+    sig = binary_type(sig)
+
+    try:
+        verifying_key.verify(sig, example_data, getattr(hashlib, hash_alg),
+                             sigdecode_der)
+        assert False
+    except BadSignatureError:
+        assert True
+
+
+####
+#
+# For string encoded signatures, only the length of string is important
+#
+####
+
+str_sigs = []
+
+#for curve in curves:
+for curve in [NIST256p]:
+    #for hash_alg in ["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]:
+    for hash_alg in ["sha256"]:
+        key = SigningKey.generate(curve)
+        signature = key.sign(example_data, hashfunc=getattr(hashlib, hash_alg),
+                             sigencode=sigencode_string)
+        for trunc in range(len(signature)):
+            str_sigs.append(pytest.param(
+                key.verifying_key, hash_alg,
+                signature, trunc,
+                id="{0}-{1}-trunc-{2}".format(
+                    curve.name, hash_alg, trunc)))
+
+
+@pytest.mark.parametrize("verifying_key,hash_alg,signature,trunc", str_sigs)
+def test_truncated_string_signatures(verifying_key, hash_alg, signature, trunc):
+    # check if a malformed string encoded signature causes the same exception
+    # to be raised irrespective of the type of error
+    sig = bytearray(signature)
+    sig = sig[:trunc]
+    sig = binary_type(sig)
+
+    try:
+        verifying_key.verify(sig, example_data, getattr(hashlib, hash_alg),
+                             sigdecode_string)
+        assert False
+    except BadSignatureError:
+        assert True

src/ecdsa/util.py

@@ -233,19 +233,39 @@ def sigencode_der_canonize(r, s, order):
     return sigencode_der(r, s, order)
 
 
+class MalformedSignature(Exception):
+    pass
+
+
 def sigdecode_string(signature, order):
     l = orderlen(order)
-    assert len(signature) == 2 * l, (len(signature), 2 * l)
+    if not len(signature) == 2 * l:
+        raise MalformedSignature(
+                "Invalid length of signature, expected {0} bytes long, "
+                "provided string is {1} bytes long"
+                .format(2 * l, len(signature)))
     r = string_to_number_fixedlen(signature[:l], order)
     s = string_to_number_fixedlen(signature[l:], order)
     return r, s
 
 
 def sigdecode_strings(rs_strings, order):
+    if not len(rs_strings) == 2:
+        raise MalformedSignature(
+                "Invalid number of strings provided: {0}, expected 2"
+                .format(len(rs_strings)))
     (r_str, s_str) = rs_strings
     l = orderlen(order)
-    assert len(r_str) == l, (len(r_str), l)
-    assert len(s_str) == l, (len(s_str), l)
+    if not len(r_str) == l:
+        raise MalformedSignature(
+                "Invalid length of first string ('r' parameter), "
+                "expected {0} bytes long, provided string is {1} bytes long"
+                .format(l, len(r_str)))
+    if not len(s_str) == l:
+        raise MalformedSignature(
+                "Invalid length of second string ('s' parameter), "
+                "expected {0} bytes long, provided string is {1} bytes long"
+                .format(l, len(s_str)))
     r = string_to_number_fixedlen(r_str, order)
     s = string_to_number_fixedlen(s_str, order)
     return r, s

tox.ini

@@ -6,6 +6,7 @@ envlist = py26, py27, py33, py34, py35, py36, py37, pypy, pypy3
 deps =
      py{33}: py<1.5
      py{33}: pytest<3.3
+     py{26}: unittest2
      py{26,27,34,35,36,37,py,py3}: pytest
      py{33}: wheel<0.30
      coverage