Browse Source

Fix potential overflows in ECDSA DER parsers

0.16
Jack Grigg 8 years ago
parent
commit
a3603ac6f0
No known key found for this signature in database
GPG Key ID: 665DBCD284F7DAFF
  1. 32
      src/key.cpp
  2. 15
      src/pubkey.cpp

32
src/key.cpp

@ -1,4 +1,5 @@
// Copyright (c) 2009-2016 The Bitcoin Core developers // Copyright (c) 2009-2016 The Bitcoin Core developers
// Copyright (c) 2017 The Zcash developers
// Distributed under the MIT software license, see the accompanying // Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php. // file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -18,41 +19,46 @@ static secp256k1_context* secp256k1_context_sign = NULL;
/** These functions are taken from the libsecp256k1 distribution and are very ugly. */ /** These functions are taken from the libsecp256k1 distribution and are very ugly. */
static int ec_privkey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *privkey, size_t privkeylen) { static int ec_privkey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *privkey, size_t privkeylen) {
const unsigned char *end = privkey + privkeylen; const unsigned char *end = privkey + privkeylen;
int lenb = 0; size_t lenb = 0;
int len = 0; size_t len = 0;
memset(out32, 0, 32); memset(out32, 0, 32);
/* sequence header */ /* sequence header */
if (end < privkey+1 || *privkey != 0x30) { if (end - privkey < 1 || *privkey != 0x30u) {
return 0; return 0;
} }
privkey++; privkey++;
/* sequence length constructor */ /* sequence length constructor */
if (end < privkey+1 || !(*privkey & 0x80)) { if (end - privkey < 1 || !(*privkey & 0x80u)) {
return 0; return 0;
} }
lenb = *privkey & ~0x80; privkey++; lenb = *privkey & ~0x80u; privkey++;
if (lenb < 1 || lenb > 2) { if (lenb < 1 || lenb > 2) {
return 0; return 0;
} }
if (end < privkey+lenb) { if (end - privkey < lenb) {
return 0; return 0;
} }
/* sequence length */ /* sequence length */
len = privkey[lenb-1] | (lenb > 1 ? privkey[lenb-2] << 8 : 0); len = privkey[lenb-1] | (lenb > 1 ? privkey[lenb-2] << 8 : 0u);
privkey += lenb; privkey += lenb;
if (end < privkey+len) { if (end - privkey < len) {
return 0; return 0;
} }
/* sequence element 0: version number (=1) */ /* sequence element 0: version number (=1) */
if (end < privkey+3 || privkey[0] != 0x02 || privkey[1] != 0x01 || privkey[2] != 0x01) { if (end - privkey < 3 || privkey[0] != 0x02u || privkey[1] != 0x01u || privkey[2] != 0x01u) {
return 0; return 0;
} }
privkey += 3; privkey += 3;
/* sequence element 1: octet string, up to 32 bytes */ /* sequence element 1: octet string, up to 32 bytes */
if (end < privkey+2 || privkey[0] != 0x04 || privkey[1] > 0x20 || end < privkey+2+privkey[1]) { if (end - privkey < 2 || privkey[0] != 0x04u) {
return 0; return 0;
} }
memcpy(out32 + 32 - privkey[1], privkey + 2, privkey[1]); size_t oslen = privkey[1];
privkey += 2;
if (oslen > 32 || end - privkey < oslen) {
return 0;
}
memcpy(out32 + (32 - oslen), privkey, oslen);
if (!secp256k1_ec_seckey_verify(ctx, out32)) { if (!secp256k1_ec_seckey_verify(ctx, out32)) {
memset(out32, 0, 32); memset(out32, 0, 32);
return 0; return 0;
@ -219,10 +225,10 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const
std::vector<unsigned char, secure_allocator<unsigned char>> vout(64); std::vector<unsigned char, secure_allocator<unsigned char>> vout(64);
if ((nChild >> 31) == 0) { if ((nChild >> 31) == 0) {
CPubKey pubkey = GetPubKey(); CPubKey pubkey = GetPubKey();
assert(pubkey.begin() + 33 == pubkey.end()); assert(pubkey.size() == 33);
BIP32Hash(cc, nChild, *pubkey.begin(), pubkey.begin()+1, vout.data()); BIP32Hash(cc, nChild, *pubkey.begin(), pubkey.begin()+1, vout.data());
} else { } else {
assert(begin() + 32 == end()); assert(size() == 32);
BIP32Hash(cc, nChild, 0, begin(), vout.data()); BIP32Hash(cc, nChild, 0, begin(), vout.data());
} }
memcpy(ccChild.begin(), vout.data()+32, 32); memcpy(ccChild.begin(), vout.data()+32, 32);

15
src/pubkey.cpp

@ -1,4 +1,5 @@
// Copyright (c) 2009-2016 The Bitcoin Core developers // Copyright (c) 2009-2016 The Bitcoin Core developers
// Copyright (c) 2017 The Zcash developers
// Distributed under the MIT software license, see the accompanying // Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php. // 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++]; lenbyte = input[pos++];
if (lenbyte & 0x80) { if (lenbyte & 0x80) {
lenbyte -= 0x80; lenbyte -= 0x80;
if (pos + lenbyte > inputlen) { if (lenbyte > inputlen - pos) {
return 0; return 0;
} }
pos += lenbyte; pos += lenbyte;
@ -65,14 +66,15 @@ static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1
lenbyte = input[pos++]; lenbyte = input[pos++];
if (lenbyte & 0x80) { if (lenbyte & 0x80) {
lenbyte -= 0x80; lenbyte -= 0x80;
if (pos + lenbyte > inputlen) { if (lenbyte > inputlen - pos) {
return 0; return 0;
} }
while (lenbyte > 0 && input[pos] == 0) { while (lenbyte > 0 && input[pos] == 0) {
pos++; pos++;
lenbyte--; lenbyte--;
} }
if (lenbyte >= sizeof(size_t)) { static_assert(sizeof(size_t) >= 4, "size_t too small");
if (lenbyte >= 4) {
return 0; return 0;
} }
rlen = 0; rlen = 0;
@ -103,14 +105,15 @@ static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1
lenbyte = input[pos++]; lenbyte = input[pos++];
if (lenbyte & 0x80) { if (lenbyte & 0x80) {
lenbyte -= 0x80; lenbyte -= 0x80;
if (pos + lenbyte > inputlen) { if (lenbyte > inputlen - pos) {
return 0; return 0;
} }
while (lenbyte > 0 && input[pos] == 0) { while (lenbyte > 0 && input[pos] == 0) {
pos++; pos++;
lenbyte--; lenbyte--;
} }
if (lenbyte >= sizeof(size_t)) { static_assert(sizeof(size_t) >= 4, "size_t too small");
if (lenbyte >= 4) {
return 0; return 0;
} }
slen = 0; slen = 0;
@ -225,7 +228,7 @@ bool CPubKey::Decompress() {
bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const { bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const {
assert(IsValid()); assert(IsValid());
assert((nChild >> 31) == 0); assert((nChild >> 31) == 0);
assert(begin() + 33 == end()); assert(size() == 33);
unsigned char out[64]; unsigned char out[64];
BIP32Hash(cc, nChild, *begin(), begin()+1, out); BIP32Hash(cc, nChild, *begin(), begin()+1, out);
memcpy(ccChild.begin(), out+32, 32); memcpy(ccChild.begin(), out+32, 32);

Loading…
Cancel
Save