Skip to content

Commit

Permalink
security: custom memcpy_safe
Browse files Browse the repository at this point in the history 
fix a number of static analysis vulnerabilities with custom memcpy_safe:
str03-c, str07-c, msc24-c, mem00-c, fio34-c cert c secure coding standard.
string termination[cjm], buffer boundary violation (buffer overflow)[hcb], unchecked array copying [xyw] iso/iec tr 24772:2013.
using a tainted value to write to an object using a formatted input or output function [taintformatio]. tainted strings are passed to a string copying function [tainstrcpy] iso/iec ts 17961:2013.
cwe-119, cwe-120, cwe-123, cwe-125, cwe-676 circa cwe 2.11.
  • Loading branch information
@xanimo
xanimo committed May 26, 2022
1 parent d427f05 commit 6a9dd0f
Show file tree
Hide file tree
Showing 27 changed files with 205 additions and 114 deletions.
Empty file removed backtrace.txt
View file
Empty file.
2 changes: 1 addition & 1 deletion include/dogecoin/crypto/hash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ LIBDOGECOIN_API static inline dogecoin_bool dogecoin_hash_equal(uint256 hash_a,

LIBDOGECOIN_API static inline void dogecoin_hash_set(uint256 hash_dest, const uint256 hash_src)
{
memcpy(hash_dest, hash_src, DOGECOIN_HASH_LENGTH);
memcpy_safe(hash_dest, hash_src, DOGECOIN_HASH_LENGTH);
}

LIBDOGECOIN_API static inline void dogecoin_hash(const unsigned char* datain, size_t length, uint256 hashout)
Expand Down
1 change: 1 addition & 0 deletions include/dogecoin/mem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ LIBDOGECOIN_API void* dogecoin_realloc(void* ptr, size_t size);
LIBDOGECOIN_API void dogecoin_free(void* ptr);

LIBDOGECOIN_API errno_t memset_safe(volatile void *v, rsize_t smax, int c, rsize_t n);
LIBDOGECOIN_API void* memcpy_safe(void* destination, const void* source, unsigned int count);
LIBDOGECOIN_API volatile void* dogecoin_mem_zero(volatile void* dst, size_t len);

LIBDOGECOIN_END_DECL
Expand Down
20 changes: 10 additions & 10 deletions src/address.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ int generatePrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testne

/* if nothing is passed in use internal variables */
if (wif_privkey) {
memcpy(wif_privkey_internal, wif_privkey, privkey_len);
memcpy_safe(wif_privkey_internal, wif_privkey, privkey_len);
}
if (p2pkh_pubkey) {
memcpy(p2pkh_pubkey_internal, p2pkh_pubkey, pubkey_len);
memcpy_safe(p2pkh_pubkey_internal, p2pkh_pubkey, pubkey_len);
}

/* determine if mainnet or testnet/regtest */
Expand All @@ -94,10 +94,10 @@ int generatePrivPubKeypair(char* wif_privkey, char* p2pkh_pubkey, bool is_testne
dogecoin_pubkey_getaddr_p2pkh(&pubkey, chain, p2pkh_pubkey_internal);

if (wif_privkey) {
memcpy(wif_privkey, wif_privkey_internal, privkey_len);
memcpy_safe(wif_privkey, wif_privkey_internal, privkey_len);
}
if (p2pkh_pubkey) {
memcpy(p2pkh_pubkey, p2pkh_pubkey_internal, pubkey_len);
memcpy_safe(p2pkh_pubkey, p2pkh_pubkey_internal, pubkey_len);
}

/* reset internal variables */
Expand Down Expand Up @@ -130,10 +130,10 @@ int generateHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_mast

/* if nothing is passed use internal variables */
if (wif_privkey_master) {
memcpy(hd_privkey_master, wif_privkey_master, strsize);
memcpy_safe(hd_privkey_master, wif_privkey_master, strsize);
}
if (p2pkh_pubkey_master) {
memcpy(hd_pubkey_master, p2pkh_pubkey_master, hd_pubkey_master_len);
memcpy_safe(hd_pubkey_master, p2pkh_pubkey_master, hd_pubkey_master_len);
}

/* determine if mainnet or testnet/regtest */
Expand All @@ -145,10 +145,10 @@ int generateHDMasterPubKeypair(char* wif_privkey_master, char* p2pkh_pubkey_mast
generateDerivedHDPubkey(hd_privkey_master, hd_pubkey_master);

if (wif_privkey_master) {
memcpy(wif_privkey_master, hd_privkey_master, strlen(hd_privkey_master));
memcpy_safe(wif_privkey_master, hd_privkey_master, strlen(hd_privkey_master));
}
if (p2pkh_pubkey_master) {
memcpy(p2pkh_pubkey_master, hd_pubkey_master, strlen(hd_pubkey_master));
memcpy_safe(p2pkh_pubkey_master, hd_pubkey_master, strlen(hd_pubkey_master));
}

/* reset internal variables */
Expand Down Expand Up @@ -184,7 +184,7 @@ int generateDerivedHDPubkey(const char* wif_privkey_master, char* p2pkh_pubkey)

/* if nothing is passed in use internal variables */
if (p2pkh_pubkey) {
memcpy(str, p2pkh_pubkey, strsize);
memcpy_safe(str, p2pkh_pubkey, strsize);
}

dogecoin_hdnode* node = dogecoin_hdnode_new();
Expand All @@ -194,7 +194,7 @@ int generateDerivedHDPubkey(const char* wif_privkey_master, char* p2pkh_pubkey)

/* pass back to external variable if exists */
if (p2pkh_pubkey) {
memcpy(p2pkh_pubkey, str, strsize);
memcpy_safe(p2pkh_pubkey, str, strsize);
}

/* reset internal variables */
Expand Down
50 changes: 25 additions & 25 deletions src/bip32.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ dogecoin_hdnode* dogecoin_hdnode_copy(const dogecoin_hdnode* hdnode)
newnode->depth = hdnode->depth;
newnode->fingerprint = hdnode->fingerprint;
newnode->child_num = hdnode->child_num;
memcpy(newnode->chain_code, hdnode->chain_code, sizeof(hdnode->chain_code));
memcpy(newnode->private_key, hdnode->private_key, sizeof(hdnode->private_key));
memcpy(newnode->public_key, hdnode->public_key, sizeof(hdnode->public_key));
memcpy_safe(newnode->chain_code, hdnode->chain_code, sizeof(hdnode->chain_code));
memcpy_safe(newnode->private_key, hdnode->private_key, sizeof(hdnode->private_key));
memcpy_safe(newnode->public_key, hdnode->public_key, sizeof(hdnode->public_key));

return newnode;
}
Expand Down Expand Up @@ -147,14 +147,14 @@ dogecoin_bool dogecoin_hdnode_from_seed(const uint8_t* seed, int seed_len, dogec
out->fingerprint = 0x00000000;
out->child_num = 0;
hmac_sha512((const uint8_t*)"Dogecoin seed", 12, seed, seed_len, I);
memcpy(out->private_key, I, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(out->private_key, I, DOGECOIN_ECKEY_PKEY_LENGTH);

if (!dogecoin_ecc_verify_privatekey(out->private_key)) {
dogecoin_mem_zero(I, sizeof(I));
return false;
}

memcpy(out->chain_code, I + DOGECOIN_ECKEY_PKEY_LENGTH, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(out->chain_code, I + DOGECOIN_ECKEY_PKEY_LENGTH, DOGECOIN_BIP32_CHAINCODE_SIZE);
dogecoin_hdnode_fill_public_key(out);
dogecoin_mem_zero(I, sizeof(I));
return true;
Expand All @@ -179,7 +179,7 @@ dogecoin_bool dogecoin_hdnode_public_ckd(dogecoin_hdnode* inout, uint32_t i)
if (i & 0x80000000) { // private derivation
return false;
} else { // public derivation
memcpy(data, inout->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(data, inout->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
}
write_be(data + DOGECOIN_ECKEY_COMPRESSED_LENGTH, i);

Expand All @@ -191,7 +191,7 @@ dogecoin_bool dogecoin_hdnode_public_ckd(dogecoin_hdnode* inout, uint32_t i)

int failed = 0;
hmac_sha512(inout->chain_code, 32, data, sizeof(data), I);
memcpy(inout->chain_code, I + 32, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(inout->chain_code, I + 32, DOGECOIN_BIP32_CHAINCODE_SIZE);


if (!dogecoin_ecc_public_key_tweak_add(inout->public_key, I))
Expand Down Expand Up @@ -229,9 +229,9 @@ dogecoin_bool dogecoin_hdnode_private_ckd(dogecoin_hdnode* inout, uint32_t i)

if (i & 0x80000000) { // private derivation
data[0] = 0;
memcpy(data + 1, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(data + 1, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
} else { // public derivation
memcpy(data, inout->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(data, inout->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
}
write_be(data + DOGECOIN_ECKEY_COMPRESSED_LENGTH, i);

Expand All @@ -241,21 +241,21 @@ dogecoin_bool dogecoin_hdnode_private_ckd(dogecoin_hdnode* inout, uint32_t i)
(fingerprint[2] << 8) + fingerprint[3];

dogecoin_mem_zero(fingerprint, sizeof(fingerprint));
memcpy(p, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(p, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);

hmac_sha512(inout->chain_code, DOGECOIN_BIP32_CHAINCODE_SIZE, data, sizeof(data), I);
memcpy(inout->chain_code, I + DOGECOIN_ECKEY_PKEY_LENGTH, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy(inout->private_key, I, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(inout->chain_code, I + DOGECOIN_ECKEY_PKEY_LENGTH, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(inout->private_key, I, DOGECOIN_ECKEY_PKEY_LENGTH);

memcpy(z, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(z, inout->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);

int failed = 0;
if (!dogecoin_ecc_verify_privatekey(z)) {
failed = 1;
return false;
}

memcpy(inout->private_key, p, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(inout->private_key, p, DOGECOIN_ECKEY_PKEY_LENGTH);
if (!dogecoin_ecc_private_key_tweak_add(inout->private_key, z)) {
failed = 1;
}
Expand Down Expand Up @@ -308,12 +308,12 @@ static void dogecoin_hdnode_serialize(const dogecoin_hdnode* node, uint32_t vers
node_data[4] = node->depth;
write_be(node_data + 5, node->fingerprint);
write_be(node_data + 9, node->child_num);
memcpy(node_data + 13, node->chain_code, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(node_data + 13, node->chain_code, DOGECOIN_BIP32_CHAINCODE_SIZE);
if (use_public) {
memcpy(node_data + 45, node->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(node_data + 45, node->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
} else {
node_data[45] = 0;
memcpy(node_data + 46, node->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(node_data + 46, node->private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
}
dogecoin_base58_encode_check(node_data, 78, str, strsize);
}
Expand Down Expand Up @@ -406,7 +406,7 @@ dogecoin_bool dogecoin_hdnode_get_pub_hex(const dogecoin_hdnode* node, char* str
{
dogecoin_pubkey pubkey;
dogecoin_pubkey_init(&pubkey);
memcpy(&pubkey.pubkey, node->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(&pubkey.pubkey, node->public_key, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
pubkey.compressed = true;

return dogecoin_pubkey_get_hex(&pubkey, str, strsize);
Expand Down Expand Up @@ -436,13 +436,13 @@ dogecoin_bool dogecoin_hdnode_deserialize(const char* str, const dogecoin_chainp
}
uint32_t version = read_be(node_data);
if (version == chain->b58prefix_bip32_pubkey) { // public node
memcpy(node->public_key, node_data + 45, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(node->public_key, node_data + 45, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
} else if (version == chain->b58prefix_bip32_privkey) { // private node
if (node_data[45]) { // invalid data
dogecoin_free(node_data);
return false;
}
memcpy(node->private_key, node_data + 46, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(node->private_key, node_data + 46, DOGECOIN_ECKEY_PKEY_LENGTH);
dogecoin_hdnode_fill_public_key(node);
} else {
dogecoin_free(node_data);
Expand All @@ -451,7 +451,7 @@ dogecoin_bool dogecoin_hdnode_deserialize(const char* str, const dogecoin_chainp
node->depth = node_data[4];
node->fingerprint = read_be(node_data + 5);
node->child_num = read_be(node_data + 9);
memcpy(node->chain_code, node_data + 13, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(node->chain_code, node_data + 13, DOGECOIN_BIP32_CHAINCODE_SIZE);
dogecoin_free(node_data);
return true;
}
Expand Down Expand Up @@ -487,7 +487,7 @@ dogecoin_bool dogecoin_hd_generate_key(dogecoin_hdnode* node, const char* keypat
}

dogecoin_mem_zero(kp, strlens(keypath) + 1);
memcpy(kp, keypath, strlens(keypath));
memcpy_safe(kp, keypath, strlens(keypath));

if (kp[0] != 'm' || kp[1] != '/') {
goto err;
Expand All @@ -496,11 +496,11 @@ dogecoin_bool dogecoin_hd_generate_key(dogecoin_hdnode* node, const char* keypat
node->depth = 0;
node->child_num = 0;
node->fingerprint = 0;
memcpy(node->chain_code, chaincode, DOGECOIN_BIP32_CHAINCODE_SIZE);
memcpy_safe(node->chain_code, chaincode, DOGECOIN_BIP32_CHAINCODE_SIZE);
if (usepubckd == true) {
memcpy(node->public_key, keymaster, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
memcpy_safe(node->public_key, keymaster, DOGECOIN_ECKEY_COMPRESSED_LENGTH);
} else {
memcpy(node->private_key, keymaster, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(node->private_key, keymaster, DOGECOIN_ECKEY_PKEY_LENGTH);
dogecoin_hdnode_fill_public_key(node);
}

Expand Down
4 changes: 2 additions & 2 deletions src/block.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ void dogecoin_block_header_serialize(cstring* s, const dogecoin_block_header* he
*/
void dogecoin_block_header_copy(dogecoin_block_header* dest, const dogecoin_block_header* src) {
dest->version = src->version;
memcpy(&dest->prev_block, &src->prev_block, sizeof(src->prev_block));
memcpy(&dest->merkle_root, &src->merkle_root, sizeof(src->merkle_root));
memcpy_safe(&dest->prev_block, &src->prev_block, sizeof(src->prev_block));
memcpy_safe(&dest->merkle_root, &src->merkle_root, sizeof(src->merkle_root));
dest->timestamp = src->timestamp;
dest->bits = src->bits;
dest->nonce = src->nonce;
Expand Down
2 changes: 1 addition & 1 deletion src/buffer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct buffer* buffer_copy(const void* data, size_t data_len)
goto err_out_free;
}

memcpy(buf->p, data, data_len);
memcpy_safe(buf->p, data, data_len);
buf->len = data_len;

return buf;
Expand Down
75 changes: 70 additions & 5 deletions src/cli/such.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -772,12 +772,77 @@ int main(int argc, char* argv[])
return showError("no derivation path (-m)");
size_t sizeout = 128;
char newextkey[sizeout];
if (!hd_derive(chain, pkey, derived_path, newextkey, sizeout))
return showError("deriving child key failed\n");
else
hd_print_node(chain, newextkey);

//check if we derive a range of keys
unsigned int maxlen = 1024;
int posanum = -1;
int posbnum = -1;
int end = -1;
uint64_t from = 0;
uint64_t to = 0;

static char digits[] = "0123456789";
unsigned int i;
for (i = 0; i < strlen(derived_path); i++) {
if (i > maxlen) {
break;
}
if (posanum > -1 && posbnum == -1) {
if (derived_path[i] == '-') {
if (i-posanum >= 9) {
break;
}
posbnum = i+1;
char buf[9] = {0};
memcpy_safe(buf, &derived_path[posanum], i-posanum);
from = strtoull(buf, NULL, 10);
} else if (!strchr(digits, derived_path[i])) {
posanum = -1;
break;
}
} else if (posanum > -1 && posbnum > -1) {
if (derived_path[i] == ']' || derived_path[i] == ')') {
if (i-posbnum >= 9) {
break;
}
char buf[9] = {0};
memcpy_safe(buf, &derived_path[posbnum], i-posbnum);
to = strtoull(buf, NULL, 10);
end = i+1;
break;
} else if (!strchr(digits, derived_path[i])) {
posbnum = -1; // value stored is never read
break;
}
}
if (derived_path[i] == '[' || derived_path[i] == '(') {
posanum = i+1;
}
}

if (end > -1 && from <= to) {
for (i = from; i <= to; i++) {
char keypathnew[strlen(derived_path)+16];
memcpy_safe(keypathnew, derived_path, posanum-1);
char index[9] = {0};
sprintf(index, "%lld", (long long)i);
memcpy_safe(keypathnew+posanum-1, index, strlen(index));
memcpy_safe(keypathnew+posanum-1+strlen(index), &derived_path[end], strlen(derived_path)-end);


if (!hd_derive(chain, pkey, keypathnew, newextkey, sizeout))
return showError("Deriving child key failed\n");
else
hd_print_node(chain, newextkey);
}
}
else {
if (!hd_derive(chain, pkey, derived_path, newextkey, sizeout))
return showError("Deriving child key failed\n");
else
hd_print_node(chain, newextkey);
}
else if (strcmp(cmd, "sign") == 0) {
} else if (strcmp(cmd, "sign") == 0) {
// ./such -c sign -x <raw hex tx> -s <script pubkey> -i <input index> -h <sighash type> -a <amount> -p <private key>
if (!txhex || !scripthex) {
return showError("Missing tx-hex or script-hex (use -x, -s)\n");
Expand Down
4 changes: 2 additions & 2 deletions src/cli/tool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ dogecoin_bool gen_privatekey(const dogecoin_chainparams* chain, char* privkey_wi
dogecoin_key key;
dogecoin_privkey_init(&key);
dogecoin_privkey_gen(&key);
memcpy(&pkeybase58c[1], key.privkey, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(&pkeybase58c[1], key.privkey, DOGECOIN_ECKEY_PKEY_LENGTH);
assert(dogecoin_base58_encode_check(pkeybase58c, 34, privkey_wif, strsize_wif) != 0);
// also export the hex privkey if use had passed in a valid pointer
// will always export 32 bytes
Expand Down Expand Up @@ -180,7 +180,7 @@ dogecoin_bool hd_print_node(const dogecoin_chainparams* chain, const char* nodes
pkeybase58c[0] = chain->b58prefix_secret_address;
pkeybase58c[33] = 1; /* always use compressed keys */
char privkey_wif[128];
memcpy(&pkeybase58c[1], node.private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
memcpy_safe(&pkeybase58c[1], node.private_key, DOGECOIN_ECKEY_PKEY_LENGTH);
assert(dogecoin_base58_encode_check(pkeybase58c, sizeof(pkeybase58c), privkey_wif, sizeof(privkey_wif)) != 0);
if (dogecoin_hdnode_has_privkey(&node)) {
printf("privatekey WIF: %s\n", privkey_wif);
Expand Down
Loading

0 comments on commit 6a9dd0f

Please sign in to comment.