From ec8f20babd8595f119e642d3833ee90bacc12873 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 7 Nov 2018 16:17:57 +0100 Subject: [PATCH] Avoid out-of-bound pointers and integer overflows in size comparisons This changes pointer calculations in size comparions to a form that ensures that no out-of-bound pointers are computed, because even their computation yields undefined behavior. Also, this changes size comparions to a form that ensures that neither the left-hand side nor the right-hand side can overflow. --- contrib/lax_der_parsing.c | 6 +++--- src/ecdsa_impl.h | 8 ++------ src/hash_impl.h | 3 ++- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/contrib/lax_der_parsing.c b/contrib/lax_der_parsing.c index 5b141a99481c7..e177a0562dd2d 100644 --- a/contrib/lax_der_parsing.c +++ b/contrib/lax_der_parsing.c @@ -32,7 +32,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_ lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } pos += lenbyte; @@ -51,7 +51,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_ lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } while (lenbyte > 0 && input[pos] == 0) { @@ -89,7 +89,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_ lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } while (lenbyte > 0 && input[pos] == 0) { diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 2a9b94a607a8b..b37aff46c0038 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -157,12 +157,8 @@ static int secp256k1_ecdsa_sig_parse(secp256k1_scalar *rr, secp256k1_scalar *rs, if (secp256k1_der_read_len(&rlen, &sig, sigend) == 0) { return 0; } - if (sig + rlen > sigend) { - /* Tuple exceeds bounds */ - return 0; - } - if (sig + rlen != sigend) { - /* Garbage after tuple. */ + if (rlen != (size_t)(sigend - sig)) { + /* Tuple exceeds bounds or garage after tuple. */ return 0; } diff --git a/src/hash_impl.h b/src/hash_impl.h index 009f26beba939..782f97216c284 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -131,7 +131,8 @@ static void secp256k1_sha256_transform(uint32_t* s, const uint32_t* chunk) { static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *data, size_t len) { size_t bufsize = hash->bytes & 0x3F; hash->bytes += len; - while (bufsize + len >= 64) { + VERIFY_CHECK(hash->bytes >= len); + while (len >= 64 - bufsize) { /* Fill the buffer, and process it. */ size_t chunk_len = 64 - bufsize; memcpy(((unsigned char*)hash->buf) + bufsize, data, chunk_len);