Merge pull request #29 from netanelkl/master

Code Security change

base58.c

@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #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)

bignum.c

@@ -26,6 +26,7 @@
 #include <assert.h>
 #include "bignum.h"
 #include "secp256k1.h"
+#include "macro_utils.h"
 
 inline uint32_t read_be(const uint8_t *data)
 {
@@ -252,6 +253,8 @@ void bn_multiply(const bignum256 *k, bignum256 *x, const bignum256 *prime)
 	for (i = 0; i < 9; i++) {
 		x->val[i] = res[i];
 	}
+
+	MEMSET_BZERO(res,sizeof(res));
 }
 
 // input x can be any normalized number that fits (0 <= x < 2^270).
@@ -309,6 +312,10 @@ void bn_sqrt(bignum256 *x, const bignum256 *prime)
 	}
 	bn_mod(&res, prime);
 	memcpy(x, &res, sizeof(bignum256));
+
+	MEMSET_BZERO(&res, sizeof(res));
+	MEMSET_BZERO(&p, sizeof(p));
+
 }
 
 #if ! USE_INVERSE_FAST
@@ -614,6 +621,11 @@ void bn_inverse(bignum256 *x, const bignum256 *prime)
 		temp32 = us.a[8-i] >> (30 - 2 * i);
 	}
 	x->val[i] = temp32;
+
+	// Let's wipe all temp buffers.
+	MEMSET_BZERO(pp, sizeof(pp));
+	MEMSET_BZERO(&us, sizeof(us));
+	MEMSET_BZERO(&vr, sizeof(vr));
 }
 #endif
 

bip32.c

@@ -31,6 +31,7 @@
 #include "sha2.h"
 #include "ripemd160.h"
 #include "base58.h"
+#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)
 {
@@ -41,7 +42,7 @@ int hdnode_from_xpub(uint32_t depth, uint32_t fingerprint, uint32_t child_num, c
 	out->fingerprint = fingerprint;
 	out->child_num = child_num;
 	memcpy(out->chain_code, chain_code, 32);
-	memset(out->private_key, 0, 32);
+	MEMSET_BZERO(out->private_key, 32);
 	memcpy(out->public_key, public_key, 33);
 	return 1;
 }
@@ -50,9 +51,20 @@ int hdnode_from_xprv(uint32_t depth, uint32_t fingerprint, uint32_t child_num, c
 {
 	bignum256 a;
 	bn_read_be(private_key, &a);
-	if (bn_is_zero(&a) || !bn_is_less(&a, &order256k1)) { // == 0 or >= order
+
+	bool failed = false;
+	if (bn_is_zero(&a)) {
+		failed = true;
+	}
+	else if( !bn_is_less(&a, &order256k1)) { // == 0 or >= order
+		MEMSET_BZERO(&a,sizeof(a));
+		failed = true;
+	}
+
+	if(failed) {
 		return 0;
 	}
+
 	out->depth = depth;
 	out->fingerprint = fingerprint;
 	out->child_num = child_num;
@@ -73,12 +85,28 @@ int hdnode_from_seed(const uint8_t *seed, int seed_len, HDNode *out)
 	memcpy(out->private_key, I, 32);
 	bignum256 a;
 	bn_read_be(out->private_key, &a);
-	if (bn_is_zero(&a) || !bn_is_less(&a, &order256k1)) { // == 0 or >= order
-		return 0;
+
+	bool failed = false;
+	if (bn_is_zero(&a)) {
+		failed = true;
 	}
-	memcpy(out->chain_code, I + 32, 32);
-	hdnode_fill_public_key(out);
-	return 1;
+	else
+	{
+		if( !bn_is_less(&a, &order256k1)) { // == 0 or >= order
+			failed = true;
+		}
+
+		// Making sure a is wiped.
+		MEMSET_BZERO(&a,sizeof(a));
+	}
+
+	if(!failed) {
+		memcpy(out->chain_code, I + 32, 32);
+		hdnode_fill_public_key(out);
+	}
+
+	MEMSET_BZERO(I,sizeof(I));
+	return failed ? 0 : 1;
 }
 
 int hdnode_private_ckd(HDNode *inout, uint32_t i)
@@ -108,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)
@@ -147,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
@@ -262,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)

ecdsa.c

@@ -33,6 +33,7 @@
 #include "hmac.h"
 #include "ecdsa.h"
 #include "base58.h"
+#include "macro_utils.h"
 
 // Set cp2 = cp1
 void point_copy(const curve_point *cp1, curve_point *cp2)
@@ -654,7 +655,11 @@ int ecdsa_sign(const uint8_t *priv_key, const uint8_t *msg, uint32_t msg_len, ui
 {
 	uint8_t hash[32];
 	sha256_Raw(msg, msg_len, hash);
-	return ecdsa_sign_digest(priv_key, hash, sig, pby);
+	int res = ecdsa_sign_digest(priv_key, hash, sig, pby);
+
+	MEMSET_BZERO(hash, sizeof(hash));
+	return res;
+
 }
 
 // msg is a data to be signed
@@ -664,7 +669,11 @@ int ecdsa_sign_double(const uint8_t *priv_key, const uint8_t *msg, uint32_t msg_
 	uint8_t hash[32];
 	sha256_Raw(msg, msg_len, hash);
 	sha256_Raw(hash, 32, hash);
-	return ecdsa_sign_digest(priv_key, hash, sig, pby);
+	int res = ecdsa_sign_digest(priv_key, hash, sig, pby);
+
+	MEMSET_BZERO(hash, sizeof(hash));
+
+	return res;
 }
 
 // uses secp256k1 curve
@@ -677,56 +686,71 @@ int ecdsa_sign_digest(const uint8_t *priv_key, const uint8_t *digest, uint8_t *s
 	curve_point R;
 	bignum256 k, z;
 	bignum256 *da = &R.y;
-
+	int result = 0;
 	bn_read_be(digest, &z);
 
 #if USE_RFC6979
 	// generate K deterministically
 	if (generate_k_rfc6979(&k, priv_key, digest) != 0) {
-		return 1;
+		result = 1;
 	}
 #else
 	// generate random number k
 	if (generate_k_random(&k) != 0) {
-		return 1;
+		result = 1;
 	}
 #endif
 
-	// compute k*G
-	scalar_multiply(&k, &R);
-	if (pby) {
-		*pby = R.y.val[0] & 1;
-	}
-	// r = (rx mod n)
-	bn_mod(&R.x, &order256k1);
-	// if r is zero, we fail
-	if (bn_is_zero(&R.x)) return 2;
-	bn_inverse(&k, &order256k1);
-	bn_read_be(priv_key, da);
-	bn_multiply(&R.x, da, &order256k1);
-	for (i = 0; i < 8; i++) {
-		da->val[i] += z.val[i];
-		da->val[i + 1] += (da->val[i] >> 30);
-		da->val[i] &= 0x3FFFFFFF;
-	}
-	da->val[8] += z.val[8];
-	bn_multiply(da, &k, &order256k1);
-	bn_mod(&k, &order256k1);
-	// if k is zero, we fail
-	if (bn_is_zero(&k)) return 3;
-
-	// if S > order/2 => S = -S
-	if (bn_is_less(&order256k1_half, &k)) {
-		bn_subtract(&order256k1, &k, &k);
+	if(result == 0)	{
+		// compute k*G
+		scalar_multiply(&k, &R);
 		if (pby) {
-			*pby = !*pby;
+			*pby = R.y.val[0] & 1;
+		}
+		// r = (rx mod n)
+		bn_mod(&R.x, &order256k1);
+		// if r is zero, we fail
+		if (bn_is_zero(&R.x))
+		{
+			result = 2;
 		}
 	}
 
-	// we are done, R.x and k is the result signature
-	bn_write_be(&R.x, sig);
-	bn_write_be(&k, sig + 32);
+	if(result == 0)	{
+		bn_inverse(&k, &order256k1);
+		bn_read_be(priv_key, da);
+		bn_multiply(&R.x, da, &order256k1);
+		for (i = 0; i < 8; i++) {
+			da->val[i] += z.val[i];
+			da->val[i + 1] += (da->val[i] >> 30);
+			da->val[i] &= 0x3FFFFFFF;
+		}
+		da->val[8] += z.val[8];
+		bn_multiply(da, &k, &order256k1);
+		bn_mod(&k, &order256k1);
+		// if k is zero, we fail
+		if (bn_is_zero(&k)) {
+			result = 3;
+		}
+	}
 
+	if(result == 0)
+	{
+		// if S > order/2 => S = -S
+		if (bn_is_less(&order256k1_half, &k)) {
+			bn_subtract(&order256k1, &k, &k);
+			if (pby) {
+				*pby = !*pby;
+			}
+		}
+		// we are done, R.x and k is the result signature
+		bn_write_be(&R.x, sig);
+		bn_write_be(&k, sig + 32);
+	}
+
+	MEMSET_BZERO(&k,sizeof(k));
+	MEMSET_BZERO(&z,sizeof(z));
+	MEMSET_BZERO(&R,sizeof(R));
 	return 0;
 }
 
@@ -740,6 +764,8 @@ void ecdsa_get_public_key33(const uint8_t *priv_key, uint8_t *pub_key)
 	scalar_multiply(&k, &R);
 	pub_key[0] = 0x02 | (R.y.val[0] & 0x01);
 	bn_write_be(&R.x, pub_key + 1);
+	MEMSET_BZERO(&R,sizeof(R));
+	MEMSET_BZERO(&k,sizeof(k));
 }
 
 void ecdsa_get_public_key65(const uint8_t *priv_key, uint8_t *pub_key)
@@ -753,6 +779,8 @@ void ecdsa_get_public_key65(const uint8_t *priv_key, uint8_t *pub_key)
 	pub_key[0] = 0x04;
 	bn_write_be(&R.x, pub_key + 1);
 	bn_write_be(&R.y, pub_key + 33);
+	MEMSET_BZERO(&R,sizeof(R));
+	MEMSET_BZERO(&k,sizeof(k));
 }
 
 void ecdsa_get_pubkeyhash(const uint8_t *pub_key, uint8_t *pubkeyhash)
@@ -766,6 +794,7 @@ void ecdsa_get_pubkeyhash(const uint8_t *pub_key, uint8_t *pubkeyhash)
 		sha256_Raw(pub_key, 33, h); // expecting compressed format
 	}
 	ripemd160(h, 32, pubkeyhash);
+	MEMSET_BZERO(h,sizeof(h));
 }
 
 void ecdsa_get_address_raw(const uint8_t *pub_key, uint8_t version, uint8_t *addr_raw)
@@ -779,6 +808,9 @@ void ecdsa_get_address(const uint8_t *pub_key, uint8_t version, char *addr, int
 	uint8_t raw[21];
 	ecdsa_get_address_raw(pub_key, version, raw);
 	base58_encode_check(raw, 21, addr, addrsize);
+
+	// Not as important to clear this one, but we might as well.
+	MEMSET_BZERO(raw,sizeof(raw));
 }
 
 void ecdsa_get_wif(const uint8_t *priv_key, uint8_t version, char *wif, int wifsize)
@@ -788,6 +820,9 @@ void ecdsa_get_wif(const uint8_t *priv_key, uint8_t version, char *wif, int wifs
 	memcpy(data + 1, priv_key, 32);
 	data[33] = 0x01;
 	base58_encode_check(data, 34, wif, wifsize);
+
+	// This private keys running around our stack can cause trouble!
+	MEMSET_BZERO(data,sizeof(data));
 }
 
 int ecdsa_address_decode(const char *addr, uint8_t *out)
@@ -871,7 +906,10 @@ int ecdsa_verify(const uint8_t *pub_key, const uint8_t *sig, const uint8_t *msg,
 {
 	uint8_t hash[32];
 	sha256_Raw(msg, msg_len, hash);
-	return ecdsa_verify_digest(pub_key, sig, hash);
+	int res = ecdsa_verify_digest(pub_key, sig, hash);
+
+	MEMSET_BZERO(hash,sizeof(hash));
+	return res;
 }
 
 int ecdsa_verify_double(const uint8_t *pub_key, const uint8_t *sig, const uint8_t *msg, uint32_t msg_len)
@@ -879,7 +917,11 @@ int ecdsa_verify_double(const uint8_t *pub_key, const uint8_t *sig, const uint8_
 	uint8_t hash[32];
 	sha256_Raw(msg, msg_len, hash);
 	sha256_Raw(hash, 32, hash);
-	return ecdsa_verify_digest(pub_key, sig, hash);
+
+	int res = ecdsa_verify_digest(pub_key, sig, hash);
+
+	MEMSET_BZERO(hash,sizeof(hash));
+	return res;
 }
 
 // returns 0 if verification succeeded
@@ -906,24 +948,36 @@ int ecdsa_verify_digest(const uint8_t *pub_key, const uint8_t *sig, const uint8_
 	bn_mod(&z, &order256k1);
 	bn_multiply(&r, &s, &order256k1); // r*s^-1
 	bn_mod(&s, &order256k1);
+
+	int result = 0;
 	if (bn_is_zero(&z)) {
 		// our message hashes to zero
 		// I don't expect this to happen any time soon
-		return 3;
+		result = 3;
 	} else {
 		scalar_multiply(&z, &res);
 	}
 
+	if(result == 0) {
+
 	// both pub and res can be infinity, can have y = 0 OR can be equal -> false negative
 	point_multiply(&s, &pub, &pub);
 	point_add(&pub, &res);
 	bn_mod(&(res.x), &order256k1);
 
 	// signature does not match
-	if (!bn_is_equal(&res.x, &r)) return 5;
+		if (!bn_is_equal(&res.x, &r)) {
+			result = 5;
+		}
+	}
 
+	MEMSET_BZERO(&pub,sizeof(pub));
+	MEMSET_BZERO(&res,sizeof(res));
+	MEMSET_BZERO(&r,sizeof(r));
+	MEMSET_BZERO(&s,sizeof(s));
+	MEMSET_BZERO(&z,sizeof(z));
 	// all OK
-	return 0;
+	return result;
 }
 
 int ecdsa_sig_to_der(const uint8_t *sig, uint8_t *der)

hmac.c

@@ -25,6 +25,7 @@
 
 #include "hmac.h"
 #include "sha2.h"
+#include "macro_utils.h"
 
 void hmac_sha256(const uint8_t *key, const uint32_t keylen, const uint8_t *msg, const uint32_t msglen, uint8_t *hmac)
 {
@@ -53,6 +54,10 @@ void hmac_sha256(const uint8_t *key, const uint32_t keylen, const uint8_t *msg,
 	sha256_Update(&ctx, o_key_pad, SHA256_BLOCK_LENGTH);
 	sha256_Update(&ctx, buf, SHA256_DIGEST_LENGTH);
 	sha256_Final(hmac, &ctx);
+	MEMSET_BZERO(buf,sizeof(buf));
+	MEMSET_BZERO(o_key_pad,sizeof(o_key_pad));
+	MEMSET_BZERO(i_key_pad,sizeof(i_key_pad));
+
 }
 
 void hmac_sha512(const uint8_t *key, const uint32_t keylen, const uint8_t *msg, const uint32_t msglen, uint8_t *hmac)
@@ -82,4 +87,8 @@ void hmac_sha512(const uint8_t *key, const uint32_t keylen, const uint8_t *msg,
 	sha512_Update(&ctx, o_key_pad, SHA512_BLOCK_LENGTH);
 	sha512_Update(&ctx, buf, SHA512_DIGEST_LENGTH);
 	sha512_Final(hmac, &ctx);
+
+	MEMSET_BZERO(buf,sizeof(buf));
+	MEMSET_BZERO(o_key_pad,sizeof(o_key_pad));
+	MEMSET_BZERO(i_key_pad,sizeof(i_key_pad));
 }

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

pbkdf2.c

@@ -24,6 +24,7 @@
 #include <string.h>
 #include "pbkdf2.h"
 #include "hmac.h"
+#include "macro_utils.h"
 
 void pbkdf2_hmac_sha256(const uint8_t *pass, int passlen, uint8_t *salt, int saltlen, uint32_t iterations, uint8_t *key, int keylen, void (*progress_callback)(uint32_t current, uint32_t total))
 {
@@ -56,6 +57,9 @@ void pbkdf2_hmac_sha256(const uint8_t *pass, int passlen, uint8_t *salt, int sal
 			memcpy(key + HMACLEN * (i - 1), f, HMACLEN);
 		}
 	}
+
+	MEMSET_BZERO(f,sizeof(f));
+	MEMSET_BZERO(g, sizeof(g));
 }
 
 void pbkdf2_hmac_sha512(const uint8_t *pass, int passlen, uint8_t *salt, int saltlen, uint32_t iterations, uint8_t *key, int keylen, void (*progress_callback)(uint32_t current, uint32_t total))
@@ -89,4 +93,7 @@ void pbkdf2_hmac_sha512(const uint8_t *pass, int passlen, uint8_t *salt, int sal
 			memcpy(key + HMACLEN * (i - 1), f, HMACLEN);
 		}
 	}
+
+	MEMSET_BZERO(f,sizeof(f));
+	MEMSET_BZERO(g, sizeof(g));
 }