mem: fix memleaks

-add dogecoin_privkey_init(&key) to verifyPrivPubKeypair to reduce error count from 624 from 35 to 21 from 13 contexts. -overhaul base58 and add double sha256 bool function reducing error count from 21 from 13 to 15 from 12 contexts. -use calloc over malloc, sizeof(hdnode) over strlen(str) to avoid conditional based on uninitialized variable. 8 errors from 5 contexts. -require p2pkh output variable on generateDerivedHDPubkey in order to unconditionally copy initialised external vars to internals. 7 errors from 4 contexts. -add string length field to utils_calculate_shannon_entropy to avoid reliance on uninitialized variables string length. 6 errors from 3 contexts. -conditional to require variables passed to verifyp2pkhaddress. 2 errors from 2 contexts. -add length to verifyp2pkhaddress. 0 errors from 0 contexts.
dogecoinfoundation · Mar 7, 2022 · 02602dd · 02602dd
1 parent d658137
commit 02602dd
Show file tree

Hide file tree

Showing 14 changed files with 190 additions and 152 deletions.
diff --git a/include/dogecoin/address.h b/include/dogecoin/address.h
@@ -38,7 +38,7 @@ LIBDOGECOIN_API int generatePrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey
 LIBDOGECOIN_API int generateHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_master, bool is_testnet);
 
 /* generate an extended public key */
-LIBDOGECOIN_API int generateDerivedHDPubkey(const char* wif_privkey_master, char* p2pkh_pubkey);
+LIBDOGECOIN_API int generateDerivedHDPubkey(char* wif_privkey_master, char* p2pkh_pubkey);
 
 /* verify private and public keys are valid and associated with each other*/
 LIBDOGECOIN_API int verifyPrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testnet);
@@ -47,7 +47,7 @@ LIBDOGECOIN_API int verifyPrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey,
 LIBDOGECOIN_API int verifyHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_master, bool is_testnet);
 
 /* verify address based on appearance only */
-LIBDOGECOIN_API int verifyP2pkhAddress(char* p2pkh_pubkey, bool is_testnet);
+LIBDOGECOIN_API int verifyP2pkhAddress(char* p2pkh_pubkey, uint8_t len, bool is_testnet);
 
 LIBDOGECOIN_END_DECL
 

diff --git a/include/dogecoin/crypto/base58.h b/include/dogecoin/crypto/base58.h
@@ -26,15 +26,22 @@
 #ifndef __LIBDOGECOIN_CRYPTO_BASE58_H__
 #define __LIBDOGECOIN_CRYPTO_BASE58_H__
 
+#include <stdint.h>
+
 #include <dogecoin/dogecoin.h>
+#include <dogecoin/chainparams.h>
 
 LIBDOGECOIN_BEGIN_DECL
 
 LIBDOGECOIN_API int dogecoin_base58_encode_check(const uint8_t* data, int len, char* str, int base58_length);
 LIBDOGECOIN_API int dogecoin_base58_decode_check(const char* str, uint8_t* data, size_t datalen);
 
 LIBDOGECOIN_API int dogecoin_base58_encode(char* b58, size_t* b58sz, const void* data, size_t binsz);
-LIBDOGECOIN_API int dogecoin_base58_decode(void* bin, size_t* binszp, const char* b58);
+LIBDOGECOIN_API int dogecoin_base58_decode(void* bin, size_t* binszp, const char* b58, size_t b58sz);
+
+LIBDOGECOIN_API dogecoin_bool dogecoin_p2pkh_addr_from_hash160(const uint160 hashin, const dogecoin_chainparams* chain, char *addrout, int len);
+LIBDOGECOIN_API dogecoin_bool dogecoin_p2sh_addr_from_hash160(const uint160 hashin, const dogecoin_chainparams* chain, char* addrout, int len);
+LIBDOGECOIN_API dogecoin_bool dogecoin_p2wpkh_addr_from_hash160(const uint160 hashin, const dogecoin_chainparams* chain, char *addrout);
 
 LIBDOGECOIN_END_DECL
 

diff --git a/include/dogecoin/crypto/hash.h b/include/dogecoin/crypto/hash.h
@@ -63,6 +63,13 @@ LIBDOGECOIN_API static inline void dogecoin_hash(const unsigned char* datain, si
     sha256_raw(hashout, SHA256_DIGEST_LENGTH, hashout); // dogecoin double sha256 hash
 }
 
+LIBDOGECOIN_API static inline dogecoin_bool dogecoin_dblhash(const unsigned char* datain, size_t length, uint256 hashout)
+{
+    sha256_raw(datain, length, hashout);
+    sha256_raw(hashout, SHA256_DIGEST_LENGTH, hashout); // dogecoin double sha256 hash
+    return true;
+}
+
 LIBDOGECOIN_API static inline void dogecoin_hash_sngl_sha256(const unsigned char* datain, size_t length, uint256 hashout)
 {
     sha256_raw(datain, length, hashout); // single sha256 hash

diff --git a/include/dogecoin/utils.h b/include/dogecoin/utils.h
@@ -48,7 +48,7 @@ LIBDOGECOIN_API uint8_t* utils_hex_to_uint8(const char* str);
 LIBDOGECOIN_API char* utils_uint8_to_hex(const uint8_t* bin, size_t l);
 LIBDOGECOIN_API void utils_reverse_hex(char* h, int len);
 LIBDOGECOIN_API void utils_uint256_sethex(char* psz, uint8_t* out);
-LIBDOGECOIN_API void utils_calculate_shannon_entropy(const char* str, double *entropy);
+LIBDOGECOIN_API void utils_calculate_shannon_entropy(const char* str, uint8_t strsz, double *entropy);
 LIBDOGECOIN_API void* safe_malloc(size_t size);
 LIBDOGECOIN_API void dogecoin_cheap_random_bytes(uint8_t* buf, uint32_t len);
 LIBDOGECOIN_API void dogecoin_get_default_datadir(cstring* path_out);

diff --git a/src/address.c b/src/address.c
@@ -91,17 +91,13 @@ int generatePrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testne
         memcpy(p2pkh_pubkey, p2pkh_pubkey_internal, sizeout);
     }
 
-    // printf("wif_privkey:  %s\n", wif_privkey);
-    // printf("p2pkh_pubkey: %s\n\n", p2pkh_pubkey);
-
     /* reset internal variables */
     memset(wif_privkey_internal, 0, strlen(wif_privkey_internal));
     memset(p2pkh_pubkey_internal, 0, strlen(p2pkh_pubkey_internal));
 
-    // TODO: evaluate how we're going to deal with key storage and cleansing memory
-    // /* cleanup memory */
-    // dogecoin_pubkey_cleanse(&pubkey);
-    // dogecoin_privkey_cleanse(&key);
+    /* cleanup memory */
+    dogecoin_pubkey_cleanse(&pubkey);
+    dogecoin_privkey_cleanse(&key);
     return true;
 }
 
@@ -141,20 +137,14 @@ int generateHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_mast
     /* reset internal variables */
     memset(hd_privkey_master, 0, strlen(hd_privkey_master));
     memset(hd_privkey_master, 0, strlen(hd_privkey_master));
-
-    // printf("wif_privkey_master:   %s\n", wif_privkey_master);
-    // printf("p2pkh_pubkey_master:  %s\n", p2pkh_pubkey_master);
-
-    // TODO: evaluate how we're going to deal with key storage and cleansing memory
-    // memset(wif_privkey_master, 0, strlen(wif_privkey_master));
-    // memset(p2pkh_pubkey_master, 0, strlen(p2pkh_pubkey_master));
+
     return true;
 }
 
-int generateDerivedHDPubkey(const char* wif_privkey_master, char* p2pkh_pubkey)
+int generateDerivedHDPubkey(char* wif_privkey_master, char* p2pkh_pubkey)
 {
     /* require master key */
-    if (!wif_privkey_master) {
+    if (!wif_privkey_master || !p2pkh_pubkey) {
         return false;
     }
 
@@ -163,26 +153,8 @@ int generateDerivedHDPubkey(const char* wif_privkey_master, char* p2pkh_pubkey)
     memcpy(prefix, wif_privkey_master, 1);
     const dogecoin_chainparams* chain = memcmp(prefix, "d", 1) == 0 ? &dogecoin_chainparams_main : &dogecoin_chainparams_test;
 
-    size_t strsize = 128;
-    char str[strsize];
-
-    /* if nothing is passed in use internal variables */
-    if (p2pkh_pubkey) {
-        memcpy(str, p2pkh_pubkey, strsize);
-    }
-
-    dogecoin_hdnode node;
-    dogecoin_hdnode_deserialize(wif_privkey_master, chain, &node);
-
-    dogecoin_hdnode_get_p2pkh_address(&node, chain, str, strsize);
-
-    /* pass back to external variable if exists */
-    if (p2pkh_pubkey) {
-        memcpy(p2pkh_pubkey, str, strsize);
-    }
-
-    /* reset internal variables */
-    memset(str, 0, strlen(str));
+    /* generate p2pkh from private key */
+    generatePrivPubKeypair(wif_privkey_master, p2pkh_pubkey, memcmp(prefix, "d", 1) == 0 ? false : true);
 
     return true;
 }
@@ -197,6 +169,7 @@ int verifyPrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testnet)
 
     /* verify private key */
     dogecoin_key key;
+    dogecoin_privkey_init(&key);
     dogecoin_privkey_decode_wif(wif_privkey, chain, &key);
     if (!dogecoin_privkey_is_valid(&key)) return false;
     char new_wif_privkey[sizeout];
@@ -213,6 +186,8 @@ int verifyPrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testnet)
     dogecoin_pubkey_getaddr_p2pkh(&pubkey, chain, new_p2pkh_pubkey);
     if (strcmp(p2pkh_pubkey, new_p2pkh_pubkey)) return false;
 
+    dogecoin_pubkey_cleanse(&pubkey);
+    dogecoin_privkey_cleanse(&key);
     return true;
 }
 
@@ -223,15 +198,6 @@ int verifyHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_master
     /* determine if mainnet or testnet/regtest */
     const dogecoin_chainparams* chain = is_testnet ? &dogecoin_chainparams_test : &dogecoin_chainparams_main;
 
-    /* validate private key */
-    // dogecoin_key key;
-    // dogecoin_privkey_init(&key);
-    // dogecoin_privkey_decode_wif(wif_privkey_master, chain, &key);
-    // if (!dogecoin_privkey_is_valid(&key)) {
-    //     printf("validate method does not apply to hd master\n");
-    //     return false;
-    // }
-
     /* calculate master pubkey from master privkey */
     dogecoin_hdnode node;
     dogecoin_hdnode_deserialize(wif_privkey_master, chain, &node);
@@ -245,17 +211,21 @@ int verifyHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_master
     return true;
 }
 
-int verifyP2pkhAddress(char* p2pkh_pubkey, bool is_testnet) {
+int verifyP2pkhAddress(char* p2pkh_pubkey, uint8_t len, bool is_testnet) {
+    if (!p2pkh_pubkey) return false;
     /* check length */
-    if (strlen(p2pkh_pubkey) != 34) return false;
-
+    if (len != 33) {
+        return false;
+    }
     /* check first character */
-    if (p2pkh_pubkey[0] != (is_testnet ? 'n' : 'D')) return false;
-
+    if (p2pkh_pubkey[0] != (is_testnet ? 110 : 68)) {
+        return false;
+    }
     /* check randomness */
     double entropy;
-    utils_calculate_shannon_entropy(p2pkh_pubkey, &entropy);
-    if (entropy < 0.12244) return false;
-
+    utils_calculate_shannon_entropy(p2pkh_pubkey, len + 1, &entropy);
+    if (entropy < 0.12244) {
+        return false;
+    }
     return true;
 }
diff --git a/src/bip32.c b/src/bip32.c
@@ -271,13 +271,11 @@ dogecoin_bool dogecoin_hdnode_get_pub_hex(const dogecoin_hdnode* node, char* str
 // check for validity of curve point in case of public data not performed
 dogecoin_bool dogecoin_hdnode_deserialize(const char* str, const dogecoin_chainparams* chain, dogecoin_hdnode* node)
 {
-    const size_t ndlen = sizeof(uint8_t) * strlen(str);
-    uint8_t* node_data = (uint8_t*)dogecoin_malloc(ndlen);
+    if (!str || !chain || !node) return false;
+    const size_t ndlen = sizeof(uint8_t) * sizeof(dogecoin_hdnode);
+    uint8_t* node_data = (uint8_t*)dogecoin_calloc(1, ndlen);
     memset(node, 0, sizeof(dogecoin_hdnode));
-    size_t outlen = 0;
-
-    outlen = dogecoin_base58_decode_check(str, node_data, ndlen);
-    if (!outlen) {
+    if (!dogecoin_base58_decode_check(str, node_data, ndlen)) {
         dogecoin_free(node_data);
         return false;
     }
@@ -310,7 +308,7 @@ dogecoin_bool dogecoin_hd_generate_key(dogecoin_hdnode* node, const char* keypat
     static char digits[] = "0123456789";
     uint64_t idx = 0;
     assert(strlens(keypath) < 1024);
-    char *pch, *kp = dogecoin_malloc(strlens(keypath) + 1);
+    char *pch, *kp = dogecoin_calloc(1, strlens(keypath) + 1);
 
     if (!kp) {
         return false;

diff --git a/src/buffer.c b/src/buffer.c
@@ -32,12 +32,12 @@ void buffer_free(void* struct_buffer)
 struct buffer* buffer_copy(const void* data, size_t data_len)
 {
     struct buffer* buf;
-    buf = dogecoin_malloc(sizeof(*buf));
+    buf = dogecoin_calloc(1, sizeof(*buf));
     if (!buf) {
         goto err_out;
     }
 
-    buf->p = dogecoin_malloc(data_len);
+    buf->p = dogecoin_calloc(1, data_len);
     if (!buf->p) {
         goto err_out_free;
     }