From 70dc71c87e5d149d2ace69ab6fac400101d384c3 Mon Sep 17 00:00:00 2001 From: netanelkl Date: Thu, 9 Apr 2015 14:14:37 -0400 Subject: [PATCH] Some more stack memory wipe before leaving functions. Note that I preferred to change the multiple returns to multiple checks of a boolean to concentrate the erase into the last part of the functions. --- base58.c | 9 +++-- bip32.c | 100 ++++++++++++++++++++++++++++++++++---------------- macro_utils.h | 8 ++++ 3 files changed, 82 insertions(+), 35 deletions(-) create mode 100644 macro_utils.h diff --git a/base58.c b/base58.c index 217264e306..444d695961 100644 --- a/base58.c +++ b/base58.c @@ -26,6 +26,7 @@ #include #include "base58.h" #include "sha2.h" +#include "macro_utils.h" static const int8_t b58digits_map[] = { -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, @@ -196,10 +197,10 @@ int base58_encode_check(const uint8_t *data, int datalen, char *str, int strsize sha256_Raw(data, datalen, hash); sha256_Raw(hash, 32, hash); size_t res = strsize; - if (b58enc(str, &res, buf, datalen + 4) != true) { - return 0; - } - return res; + bool fSuccess = b58enc(str, &res, buf, datalen + 4); + + MEMSET_BZERO(buf, sizeof(buf)); + return fSuccess ? res : 0; } int base58_decode_check(const char *str, uint8_t *data, int datalen) diff --git a/bip32.c b/bip32.c index a04ae7fcb1..4137e6818d 100644 --- a/bip32.c +++ b/bip32.c @@ -31,8 +31,7 @@ #include "sha2.h" #include "ripemd160.h" #include "base58.h" - -#define MEMSET_BZERO(p,l) memset((p), 0, (l)) +#include "macro_utils.h" int hdnode_from_xpub(uint32_t depth, uint32_t fingerprint, uint32_t child_num, const uint8_t *chain_code, const uint8_t *public_key, HDNode *out) { @@ -91,9 +90,14 @@ int hdnode_from_seed(const uint8_t *seed, int seed_len, HDNode *out) if (bn_is_zero(&a)) { failed = true; } - else if( !bn_is_less(&a, &order256k1)) { // == 0 or >= order + else + { + if( !bn_is_less(&a, &order256k1)) { // == 0 or >= order + failed = true; + } + + // Making sure a is wiped. MEMSET_BZERO(&a,sizeof(a)); - failed = true; } if(!failed) { @@ -132,23 +136,36 @@ int hdnode_private_ckd(HDNode *inout, uint32_t i) bn_read_be(inout->private_key, &b); + bool failed = false; + if (!bn_is_less(&b, &order256k1)) { // >= order - return 0; + failed = true; + } + if(!failed) { + + bn_addmod(&a, &b, &order256k1); + + if (bn_is_zero(&a)) { + failed = true; + } } - bn_addmod(&a, &b, &order256k1); + if(!failed) + { + inout->depth++; + inout->child_num = i; + bn_write_be(&a, inout->private_key); - if (bn_is_zero(&a)) { - return 0; + hdnode_fill_public_key(inout); } - inout->depth++; - inout->child_num = i; - bn_write_be(&a, inout->private_key); - - hdnode_fill_public_key(inout); - - return 1; + // Making sure to wipe our memory! + MEMSET_BZERO(&a,sizeof(a)); + MEMSET_BZERO(&b,sizeof(b)); + MEMSET_BZERO(I,sizeof(I)); + MEMSET_BZERO(fingerprint,sizeof(fingerprint)); + MEMSET_BZERO(data,sizeof(data)); + return failed ? 0 : 1; } int hdnode_public_ckd(HDNode *inout, uint32_t i) @@ -171,32 +188,51 @@ int hdnode_public_ckd(HDNode *inout, uint32_t i) inout->fingerprint = (fingerprint[0] << 24) + (fingerprint[1] << 16) + (fingerprint[2] << 8) + fingerprint[3]; memset(inout->private_key, 0, 32); + + bool failed = false; if (!ecdsa_read_pubkey(inout->public_key, &a)) { - return 0; + failed = true; } - hmac_sha512(inout->chain_code, 32, data, sizeof(data), I); - memcpy(inout->chain_code, I + 32, 32); - bn_read_be(I, &c); + if(!failed) + { + hmac_sha512(inout->chain_code, 32, data, sizeof(data), I); + memcpy(inout->chain_code, I + 32, 32); + bn_read_be(I, &c); - if (!bn_is_less(&c, &order256k1)) { // >= order - return 0; + if (!bn_is_less(&c, &order256k1)) { // >= order + failed = true; + } } - scalar_multiply(&c, &b); // b = c * G - point_add(&a, &b); // b = a + b + if(!failed) + { + scalar_multiply(&c, &b); // b = c * G + point_add(&a, &b); // b = a + b - if (!ecdsa_validate_pubkey(&b)) { - return 0; + if (!ecdsa_validate_pubkey(&b)) { + failed = true; + } } - inout->public_key[0] = 0x02 | (b.y.val[0] & 0x01); - bn_write_be(&b.x, inout->public_key + 1); + if(!failed) + { + inout->public_key[0] = 0x02 | (b.y.val[0] & 0x01); + bn_write_be(&b.x, inout->public_key + 1); - inout->depth++; - inout->child_num = i; + inout->depth++; + inout->child_num = i; + } - return 1; + // Wipe all stack data. + MEMSET_BZERO(data,sizeof(data)); + MEMSET_BZERO(I,sizeof(I)); + MEMSET_BZERO(fingerprint,sizeof(fingerprint)); + MEMSET_BZERO(&a,sizeof(a)); + MEMSET_BZERO(&b,sizeof(b)); + MEMSET_BZERO(&c,sizeof(c)); + + return failed ? 0 : 1; } #if USE_BIP32_CACHE @@ -286,7 +322,9 @@ void hdnode_serialize(const HDNode *node, uint32_t version, char use_public, cha node_data[45] = 0; memcpy(node_data + 46, node->private_key, 32); } - base58_encode_check(node_data, 78, str, strsize); + base58_encode_check(node_data, sizeof(node_data), str, strsize); + + MEMSET_BZERO(node_data, sizeof(node_data)); } void hdnode_serialize_public(const HDNode *node, char *str, int strsize) diff --git a/macro_utils.h b/macro_utils.h new file mode 100644 index 0000000000..54634f8d18 --- /dev/null +++ b/macro_utils.h @@ -0,0 +1,8 @@ + +#if !defined( _MACRO_UTILS_H ) +#define _MACRO_UTILS_H + +#define MEMSET_BZERO(p,l) memset((p), 0, (l)) + + +#endif