From 3cb057f8429c812b5dbfcd43299658463162b740 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 1 Nov 2018 12:15:28 +0100 Subject: [PATCH] Fix possible integer overflow in DER parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we’re in the last loop iteration, then `lenleft == 1` and it could be the case that `ret == MAX_SIZE`, and so `ret + lenleft` will overflow to 0 and the sanity check will not catch it. Then we will return `(int) MAX_SIZE`, which should be avoided because this value is implementation-defined. (However, this is harmless because `(int) MAX_SIZE == -1` on all supported platforms.) --- src/ecdsa_impl.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index c3400042d8393..4f62198b85206 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -66,7 +66,7 @@ static int secp256k1_der_read_len(const unsigned char **sigp, const unsigned cha return -1; } /* X.690-207 8.1.3.5 long form length octets */ - lenleft = b1 & 0x7F; + lenleft = b1 & 0x7F; /* lenleft is at least 1 */ if (lenleft > sigend - *sigp) { return -1; } @@ -82,13 +82,13 @@ static int secp256k1_der_read_len(const unsigned char **sigp, const unsigned cha } while (lenleft > 0) { ret = (ret << 8) | **sigp; - if (ret + lenleft > (size_t)(sigend - *sigp)) { - /* Result exceeds the length of the passed array. */ - return -1; - } (*sigp)++; lenleft--; } + if (ret > (size_t)(sigend - *sigp)) { + /* Result exceeds the length of the passed array. */ + return -1; + } if (ret < 128) { /* Not the shortest possible length encoding. */ return -1;