From a3603ac6f07966036e56554cd754a57791a3491a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 3 May 2017 00:14:55 +1200 Subject: Fix potential overflows in ECDSA DER parsers --- src/pubkey.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'src/pubkey.cpp') diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 91af4e56f..da02fec7a 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -1,4 +1,5 @@ // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2017 The Zcash developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -46,7 +47,7 @@ static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1 lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } pos += lenbyte; @@ -65,14 +66,15 @@ static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1 lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } while (lenbyte > 0 && input[pos] == 0) { pos++; lenbyte--; } - if (lenbyte >= sizeof(size_t)) { + static_assert(sizeof(size_t) >= 4, "size_t too small"); + if (lenbyte >= 4) { return 0; } rlen = 0; @@ -103,14 +105,15 @@ static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1 lenbyte = input[pos++]; if (lenbyte & 0x80) { lenbyte -= 0x80; - if (pos + lenbyte > inputlen) { + if (lenbyte > inputlen - pos) { return 0; } while (lenbyte > 0 && input[pos] == 0) { pos++; lenbyte--; } - if (lenbyte >= sizeof(size_t)) { + static_assert(sizeof(size_t) >= 4, "size_t too small"); + if (lenbyte >= 4) { return 0; } slen = 0; @@ -225,7 +228,7 @@ bool CPubKey::Decompress() { bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const { assert(IsValid()); assert((nChild >> 31) == 0); - assert(begin() + 33 == end()); + assert(size() == 33); unsigned char out[64]; BIP32Hash(cc, nChild, *begin(), begin()+1, out); memcpy(ccChild.begin(), out+32, 32); -- cgit v1.2.3 From 17fa3913ef7ba5f569ebc3695fab15b10dd914f0 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 6 Jun 2017 19:21:34 +1200 Subject: Specify ECDSA constant sizes as constants --- src/pubkey.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'src/pubkey.cpp') diff --git a/src/pubkey.cpp b/src/pubkey.cpp index da02fec7a..7e7b15922 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -185,7 +185,7 @@ bool CPubKey::Verify(const uint256 &hash, const std::vector& vchS } bool CPubKey::RecoverCompact(const uint256 &hash, const std::vector& vchSig) { - if (vchSig.size() != 65) + if (vchSig.size() != COMPACT_SIGNATURE_SIZE) return false; int recid = (vchSig[0] - 27) & 3; bool fComp = ((vchSig[0] - 27) & 4) != 0; @@ -197,8 +197,8 @@ bool CPubKey::RecoverCompact(const uint256 &hash, const std::vector> 31) == 0); - assert(size() == 33); + assert(size() == COMPRESSED_PUBLIC_KEY_SIZE); unsigned char out[64]; BIP32Hash(cc, nChild, *begin(), begin()+1, out); memcpy(ccChild.begin(), out+32, 32); @@ -239,8 +239,8 @@ bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChi if (!secp256k1_ec_pubkey_tweak_add(secp256k1_context_verify, &pubkey, out)) { return false; } - unsigned char pub[33]; - size_t publen = 33; + unsigned char pub[COMPRESSED_PUBLIC_KEY_SIZE]; + size_t publen = COMPRESSED_PUBLIC_KEY_SIZE; secp256k1_ec_pubkey_serialize(secp256k1_context_verify, pub, &publen, &pubkey, SECP256K1_EC_COMPRESSED); pubkeyChild.Set(pub, pub + publen); return true; @@ -252,8 +252,8 @@ void CExtPubKey::Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const { code[5] = (nChild >> 24) & 0xFF; code[6] = (nChild >> 16) & 0xFF; code[7] = (nChild >> 8) & 0xFF; code[8] = (nChild >> 0) & 0xFF; memcpy(code+9, chaincode.begin(), 32); - assert(pubkey.size() == 33); - memcpy(code+41, pubkey.begin(), 33); + assert(pubkey.size() == COMPRESSED_PUBLIC_KEY_SIZE); + memcpy(code+41, pubkey.begin(), COMPRESSED_PUBLIC_KEY_SIZE); } void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { -- cgit v1.2.3 From 63179d028347bf3e32c7ea61386df4c44307b4a7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 4 Oct 2017 14:41:40 +0100 Subject: Scope the ECDSA constant sizes to CPubKey / CKey classes --- src/pubkey.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/pubkey.cpp') diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 7e7b15922..297c60fb3 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -252,8 +252,8 @@ void CExtPubKey::Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const { code[5] = (nChild >> 24) & 0xFF; code[6] = (nChild >> 16) & 0xFF; code[7] = (nChild >> 8) & 0xFF; code[8] = (nChild >> 0) & 0xFF; memcpy(code+9, chaincode.begin(), 32); - assert(pubkey.size() == COMPRESSED_PUBLIC_KEY_SIZE); - memcpy(code+41, pubkey.begin(), COMPRESSED_PUBLIC_KEY_SIZE); + assert(pubkey.size() == CPubKey::COMPRESSED_PUBLIC_KEY_SIZE); + memcpy(code+41, pubkey.begin(), CPubKey::COMPRESSED_PUBLIC_KEY_SIZE); } void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { -- cgit v1.2.3