From 77668b9e39e647a5c2d9337024504398f29664db Mon Sep 17 00:00:00 2001 From: Duc Nguyen Date: Thu, 19 Oct 2023 23:41:45 -0400 Subject: [PATCH] convert all variable length array to malloc/free fix astyle fixed all memory errors --- src/sig_stfl/xmss/external/hash.c | 41 ++++++-- src/sig_stfl/xmss/external/utils.h | 2 +- src/sig_stfl/xmss/external/wots.c | 25 +++-- src/sig_stfl/xmss/external/xmss_commons.c | 28 +++-- src/sig_stfl/xmss/external/xmss_core_fast.c | 107 ++++++++++++-------- 5 files changed, 133 insertions(+), 70 deletions(-) diff --git a/src/sig_stfl/xmss/external/hash.c b/src/sig_stfl/xmss/external/hash.c index c335d7d680..a6bac00724 100644 --- a/src/sig_stfl/xmss/external/hash.c +++ b/src/sig_stfl/xmss/external/hash.c @@ -30,13 +30,17 @@ int prf(const xmss_params *params, unsigned char *out, const unsigned char in[32], const unsigned char *key) { - unsigned char buf[params->padding_len + params->n + 32]; + unsigned char* buf = malloc(params->padding_len + params->n + 32); ull_to_bytes(buf, params->padding_len, XMSS_HASH_PADDING_PRF); memcpy(buf + params->padding_len, key, params->n); memcpy(buf + params->padding_len + params->n, in, 32); - return core_hash(params, out, buf, params->padding_len + params->n + 32); + int ret = core_hash(params, out, buf, params->padding_len + params->n + 32); + + OQS_MEM_insecure_free(buf); + + return ret; } /* @@ -47,13 +51,17 @@ int prf_keygen(const xmss_params *params, unsigned char *out, const unsigned char *in, const unsigned char *key) { - unsigned char buf[params->padding_len + 2*params->n + 32]; + unsigned char *buf = malloc(params->padding_len + 2*params->n + 32); ull_to_bytes(buf, params->padding_len, XMSS_HASH_PADDING_PRF_KEYGEN); memcpy(buf + params->padding_len, key, params->n); memcpy(buf + params->padding_len + params->n, in, params->n + 32); - return core_hash(params, out, buf, params->padding_len + 2*params->n + 32); + int ret = core_hash(params, out, buf, params->padding_len + 2*params->n + 32); + + OQS_MEM_insecure_free(buf); + + return ret; } /* @@ -85,8 +93,11 @@ int thash_h(const xmss_params *params, unsigned char *out, const unsigned char *in, const unsigned char *pub_seed, uint32_t addr[8]) { - unsigned char buf[params->padding_len + 3 * params->n]; - unsigned char bitmask[2 * params->n]; + unsigned char *tmp = malloc(params->padding_len + 3 * params->n + 2 * params->n); + + unsigned char *buf = tmp; + unsigned char *bitmask = tmp + (params->padding_len + 3 * params->n); + unsigned char addr_as_bytes[32]; unsigned int i; @@ -110,15 +121,21 @@ int thash_h(const xmss_params *params, for (i = 0; i < 2 * params->n; i++) { buf[params->padding_len + params->n + i] = in[i] ^ bitmask[i]; } - return core_hash(params, out, buf, params->padding_len + 3 * params->n); + int ret = core_hash(params, out, buf, params->padding_len + 3 * params->n); + + OQS_MEM_insecure_free(tmp); + + return ret; } int thash_f(const xmss_params *params, unsigned char *out, const unsigned char *in, const unsigned char *pub_seed, uint32_t addr[8]) { - unsigned char buf[params->padding_len + 2 * params->n]; - unsigned char bitmask[params->n]; + unsigned char *tmp = malloc(params->padding_len + 2 * params->n + params->n); + unsigned char *buf = tmp; + unsigned char *bitmask = tmp + (params->padding_len + 2 * params->n); + unsigned char addr_as_bytes[32]; unsigned int i; @@ -138,5 +155,9 @@ int thash_f(const xmss_params *params, for (i = 0; i < params->n; i++) { buf[params->padding_len + params->n + i] = in[i] ^ bitmask[i]; } - return core_hash(params, out, buf, params->padding_len + 2 * params->n); + int ret = core_hash(params, out, buf, params->padding_len + 2 * params->n); + + OQS_MEM_insecure_free(tmp); + + return ret; } diff --git a/src/sig_stfl/xmss/external/utils.h b/src/sig_stfl/xmss/external/utils.h index 0cdf79475a..fc5df634a6 100644 --- a/src/sig_stfl/xmss/external/utils.h +++ b/src/sig_stfl/xmss/external/utils.h @@ -2,7 +2,7 @@ #define XMSS_UTILS_H #include "namespace.h" - +#include /** * Converts the value of 'in' to 'outlen' bytes in big-endian byte order. */ diff --git a/src/sig_stfl/xmss/external/wots.c b/src/sig_stfl/xmss/external/wots.c index 90a6bd74d0..09db90e55c 100644 --- a/src/sig_stfl/xmss/external/wots.c +++ b/src/sig_stfl/xmss/external/wots.c @@ -12,11 +12,11 @@ * Expands an n-byte array into a len*n byte array using the `prf_keygen` function. */ static void expand_seed(const xmss_params *params, - unsigned char *outseeds, const unsigned char *inseed, + unsigned char *outseeds, const unsigned char *inseed, const unsigned char *pub_seed, uint32_t addr[8]) { unsigned int i; - unsigned char buf[params->n + 32]; + unsigned char *buf = malloc(params->n + 32); set_hash_addr(addr, 0); set_key_and_mask(addr, 0); @@ -26,6 +26,8 @@ static void expand_seed(const xmss_params *params, addr_to_bytes(buf + params->n, addr); prf_keygen(params, outseeds + i*params->n, buf, inseed); } + + OQS_MEM_insecure_free(buf); } /** @@ -83,7 +85,8 @@ static void wots_checksum(const xmss_params *params, unsigned int *csum_base_w, const unsigned int *msg_base_w) { int csum = 0; - unsigned char csum_bytes[(params->wots_len2 * params->wots_log_w + 7) / 8]; + unsigned int csum_bytes_length = (params->wots_len2 * params->wots_log_w + 7) / 8; + unsigned char *csum_bytes = malloc(csum_bytes_length); unsigned int i; /* Compute checksum. */ @@ -94,8 +97,10 @@ static void wots_checksum(const xmss_params *params, /* Convert checksum to base_w. */ /* Make sure expected empty zero bits are the least significant bits. */ csum = csum << (8 - ((params->wots_len2 * params->wots_log_w) % 8)); - ull_to_bytes(csum_bytes, sizeof(csum_bytes), csum); + ull_to_bytes(csum_bytes, csum_bytes_length, csum); base_w(params, csum_base_w, params->wots_len2, csum_bytes); + + OQS_MEM_insecure_free(csum_bytes); } /* Takes a message and derives the matching chain lengths. */ @@ -139,11 +144,9 @@ void wots_sign(const xmss_params *params, const unsigned char *seed, const unsigned char *pub_seed, uint32_t addr[8]) { - unsigned int lengths[params->wots_len]; + unsigned int *lengths = calloc(params->wots_len, sizeof(unsigned int)); unsigned int i; - memset(lengths, 0, sizeof(unsigned int)*params->wots_len); - chain_lengths(params, lengths, msg); /* The WOTS+ private key is derived from the seed. */ @@ -154,6 +157,8 @@ void wots_sign(const xmss_params *params, gen_chain(params, sig + i*params->n, sig + i*params->n, 0, lengths[i], pub_seed, addr); } + + OQS_MEM_insecure_free(lengths); } /** @@ -165,11 +170,9 @@ void wots_pk_from_sig(const xmss_params *params, unsigned char *pk, const unsigned char *sig, const unsigned char *msg, const unsigned char *pub_seed, uint32_t addr[8]) { - unsigned int lengths[params->wots_len]; + unsigned int *lengths = calloc(params->wots_len, sizeof(unsigned int )); unsigned int i; - memset(lengths, 0, sizeof(unsigned int)*params->wots_len); - chain_lengths(params, lengths, msg); for (i = 0; i < params->wots_len; i++) { @@ -177,4 +180,6 @@ void wots_pk_from_sig(const xmss_params *params, unsigned char *pk, gen_chain(params, pk + i*params->n, sig + i*params->n, lengths[i], params->wots_w - 1 - lengths[i], pub_seed, addr); } + + OQS_MEM_insecure_free(lengths); } diff --git a/src/sig_stfl/xmss/external/xmss_commons.c b/src/sig_stfl/xmss/external/xmss_commons.c index 882a3e39d6..9838f755b0 100644 --- a/src/sig_stfl/xmss/external/xmss_commons.c +++ b/src/sig_stfl/xmss/external/xmss_commons.c @@ -57,7 +57,7 @@ static void compute_root(const xmss_params *params, unsigned char *root, const unsigned char *pub_seed, uint32_t addr[8]) { uint32_t i; - unsigned char buffer[2*params->n]; + unsigned char *buffer = malloc(2*params->n); /* If leafidx is odd (last bit = 1), current path element is a right child and auth_path has to go left. Otherwise it is the other way around. */ @@ -93,6 +93,8 @@ static void compute_root(const xmss_params *params, unsigned char *root, leafidx >>= 1; set_tree_index(addr, leafidx); thash_h(params, root, buffer, pub_seed, addr); + + OQS_MEM_insecure_free(buffer); } @@ -105,11 +107,13 @@ void gen_leaf_wots(const xmss_params *params, unsigned char *leaf, const unsigned char *sk_seed, const unsigned char *pub_seed, uint32_t ltree_addr[8], uint32_t ots_addr[8]) { - unsigned char pk[params->wots_sig_bytes]; + unsigned char *pk = malloc(params->wots_sig_bytes); wots_pkgen(params, pk, sk_seed, pub_seed, ots_addr); l_tree(params, leaf, pk, pub_seed, ltree_addr); + + OQS_MEM_insecure_free(pk); } @@ -140,16 +144,18 @@ int xmssmt_core_sign_open(const xmss_params *params, { const unsigned char *pub_root = pk; const unsigned char *pub_seed = pk + params->n; - unsigned char wots_pk[params->wots_sig_bytes]; - unsigned char leaf[params->n]; - unsigned char root[params->n]; + + unsigned char *tmp = malloc(params->wots_sig_bytes + params->n + params->n); + unsigned char *wots_pk = tmp; + unsigned char *leaf = tmp + params->wots_sig_bytes; + unsigned char *root = leaf + params->n; unsigned long long prefix_length = params->padding_len + 3*params->n; unsigned char m_with_prefix[mlen + prefix_length]; - + unsigned char *mhash = root; unsigned long long idx = 0; - unsigned int i; + unsigned int i, ret; uint32_t idx_leaf; uint32_t ots_addr[8] = {0}; @@ -209,8 +215,12 @@ int xmssmt_core_sign_open(const xmss_params *params, /* Check if the root node equals the root node in the public key. */ if (memcmp(root, pub_root, params->n)) { /* If not, return fail */ - return -1; + ret = -1; + goto fail; } + ret = 0; +fail: + OQS_MEM_insecure_free(tmp); + return ret; - return 0; } diff --git a/src/sig_stfl/xmss/external/xmss_core_fast.c b/src/sig_stfl/xmss/external/xmss_core_fast.c index b3de5f17f0..78dd95ec1d 100644 --- a/src/sig_stfl/xmss/external/xmss_core_fast.c +++ b/src/sig_stfl/xmss/external/xmss_core_fast.c @@ -170,11 +170,11 @@ static void deep_state_swap(const xmss_params *params, } // TODO this is extremely ugly and should be refactored // TODO right now, this ensures that both 'stack' and 'retain' fit - unsigned char t[ + unsigned char *t = malloc( ((params->tree_height + 1) > ((1 << params->bds_k) - params->bds_k - 1) ? (params->tree_height + 1) : ((1 << params->bds_k) - params->bds_k - 1)) - * params->n]; + * params->n); unsigned int i; memswap(a->stack, b->stack, t, (params->tree_height + 1) * params->n); @@ -193,6 +193,8 @@ static void deep_state_swap(const xmss_params *params, memswap(a->retain, b->retain, t, ((1 << params->bds_k) - params->bds_k - 1) * params->n); memswap(&a->next_leaf, &b->next_leaf, t, sizeof(a->next_leaf)); + + OQS_MEM_insecure_free(t); } static int treehash_minheight_on_stack(const xmss_params *params, @@ -235,7 +237,7 @@ static void treehash_init(const xmss_params *params, uint32_t lastnode, i; unsigned char *stack = calloc((height+1)*params->n, sizeof(unsigned char)); - unsigned int stacklevels[height+1]; + unsigned int *stacklevels = malloc((height + 1)*sizeof(unsigned int)); unsigned int stackoffset=0; unsigned int nodeh; @@ -283,6 +285,7 @@ static void treehash_init(const xmss_params *params, node[i] = stack[i]; } + OQS_MEM_insecure_free(stacklevels); OQS_MEM_insecure_free(stack); } @@ -307,7 +310,7 @@ static void treehash_update(const xmss_params *params, set_ltree_addr(ltree_addr, treehash->next_idx); set_ots_addr(ots_addr, treehash->next_idx); - unsigned char nodebuffer[2 * params->n]; + unsigned char *nodebuffer = malloc(2 * params->n); unsigned int nodeheight = 0; gen_leaf_wots(params, nodebuffer, sk_seed, pub_seed, ltree_addr, ots_addr); while (treehash->stackusage > 0 && state->stacklevels[state->stackoffset-1] == nodeheight) { @@ -331,6 +334,8 @@ static void treehash_update(const xmss_params *params, state->stackoffset++; treehash->next_idx++; } + + OQS_MEM_insecure_free(nodebuffer); } /** @@ -454,7 +459,7 @@ static void bds_round(const xmss_params *params, unsigned int tau = params->tree_height; unsigned int startidx; unsigned int offset, rowidx; - unsigned char buf[2 * params->n]; + unsigned char *buf = malloc(2 * params->n); uint32_t ots_addr[8] = {0}; uint32_t ltree_addr[8] = {0}; @@ -514,6 +519,8 @@ static void bds_round(const xmss_params *params, } } } + + OQS_MEM_insecure_free(buf); } /** @@ -551,7 +558,7 @@ int xmss_core_keypair(const xmss_params *params, // TODO refactor BDS state not to need separate treehash instances bds_state state; - treehash_inst treehash[params->tree_height - params->bds_k]; + treehash_inst *treehash = calloc(params->tree_height - params->bds_k, sizeof(treehash_inst)); state.treehash = treehash; xmss_deserialize_state(params, &state, sk); @@ -580,6 +587,8 @@ int xmss_core_keypair(const xmss_params *params, /* Write the BDS state into sk. */ xmss_serialize_state(params, sk, &state); + OQS_MEM_insecure_free(treehash); + return 0; } @@ -601,12 +610,13 @@ int xmss_core_sign(const xmss_params *params, } const unsigned char *pub_root = sk + params->index_bytes + 2*params->n; + int ret; uint16_t i = 0; // TODO refactor BDS state not to need separate treehash instances bds_state state; - treehash_inst treehash[params->tree_height - params->bds_k]; + treehash_inst *treehash = calloc(params->tree_height - params->bds_k, sizeof(treehash_inst)); state.treehash = treehash; /* Load the BDS state from sk. */ @@ -617,29 +627,32 @@ int xmss_core_sign(const xmss_params *params, /* Check if we can still sign with this sk. * If not, return -2 - * - * If this is the last possible signature (because the max index value - * is reached), production implementations should delete the secret key + * + * If this is the last possible signature (because the max index value + * is reached), production implementations should delete the secret key * to prevent accidental further use. - * - * For the case of total tree height of 64 we do not use the last signature - * to be on the safe side (there is no index value left to indicate that the + * + * For the case of total tree height of 64 we do not use the last signature + * to be on the safe side (there is no index value left to indicate that the * key is finished, hence external handling would be necessary) - */ + */ if (idx >= ((1ULL << params->full_height) - 1)) { // Delete secret key here. We only do this in memory, production code // has to make sure that this happens on disk. memset(sk, 0xFF, params->index_bytes); memset(sk + params->index_bytes, 0, (params->sk_bytes - params->index_bytes)); if (idx > ((1ULL << params->full_height) - 1)) - return -2; // We already used all one-time keys + ret = -2; // We already used all one-time keys + goto cleanup; } - - unsigned char sk_seed[params->n]; + unsigned char *tmp = malloc(5 * params->n); + + unsigned char *sk_seed = tmp; + unsigned char *sk_prf = sk_seed + params->n; + unsigned char *pub_seed = sk_prf + params->n; + memcpy(sk_seed, sk + params->index_bytes, params->n); - unsigned char sk_prf[params->n]; memcpy(sk_prf, sk + params->index_bytes + params->n, params->n); - unsigned char pub_seed[params->n]; memcpy(pub_seed, sk + params->index_bytes + 3*params->n, params->n); // index as 32 bytes string @@ -656,8 +669,8 @@ int xmss_core_sign(const xmss_params *params, // and write the updated secret key at this point! // Init working params - unsigned char R[params->n]; - unsigned char msg_h[params->n]; + unsigned char *R = pub_seed + params->n; + unsigned char *msg_h = R + params->n; uint32_t ots_addr[8] = {0}; // --------------------------------- @@ -671,7 +684,7 @@ int xmss_core_sign(const xmss_params *params, /* Already put the message in the right place, to make it easier to prepend * things when computing the hash over the message. */ unsigned long long prefix_length = params->padding_len + 3*params->n; - unsigned char m_with_prefix[mlen + prefix_length]; + unsigned char *m_with_prefix = malloc(mlen + prefix_length); memcpy(m_with_prefix, sm + params->sig_bytes - prefix_length, prefix_length); memcpy(m_with_prefix + prefix_length, m, mlen); @@ -727,7 +740,15 @@ int xmss_core_sign(const xmss_params *params, /* Write the updated BDS state back into sk. */ xmss_serialize_state(params, sk, &state); - return 0; + ret = 0; + + OQS_MEM_insecure_free(m_with_prefix); + OQS_MEM_insecure_free(tmp); + +cleanup: + OQS_MEM_insecure_free(treehash); + + return ret; } /* @@ -743,8 +764,8 @@ int xmssmt_core_keypair(const xmss_params *params, unsigned char *wots_sigs; // TODO refactor BDS state not to need separate treehash instances - bds_state states[2*params->d - 1]; - treehash_inst treehash[(2*params->d - 1) * (params->tree_height - params->bds_k)]; + bds_state *states = calloc(2*params->d - 1, sizeof(bds_state)); + treehash_inst *treehash = calloc((2*params->d - 1) * (params->tree_height - params->bds_k), sizeof(treehash_inst)); for (i = 0; i < 2*params->d - 1; i++) { states[i].treehash = treehash + i * (params->tree_height - params->bds_k); } @@ -783,6 +804,9 @@ int xmssmt_core_keypair(const xmss_params *params, xmssmt_serialize_state(params, sk, states); + OQS_MEM_insecure_free(treehash); + OQS_MEM_insecure_free(states); + return 0; } @@ -811,12 +835,14 @@ int xmssmt_core_sign(const xmss_params *params, int needswap_upto = -1; unsigned int updates; - unsigned char sk_seed[params->n]; - unsigned char sk_prf[params->n]; - unsigned char pub_seed[params->n]; + unsigned char *tmp = malloc(5 * params->n); + + unsigned char *sk_seed = tmp; + unsigned char *sk_prf = sk_seed + params->n; + unsigned char *pub_seed = sk_prf + params->n; // Init working params - unsigned char R[params->n]; - unsigned char msg_h[params->n]; + unsigned char *R = pub_seed + params->n; + unsigned char *msg_h = R + params->n; uint32_t addr[8] = {0}; uint32_t ots_addr[8] = {0}; unsigned char idx_bytes_32[32]; @@ -828,7 +854,7 @@ int xmssmt_core_sign(const xmss_params *params, // TODO refactor BDS state not to need separate treehash instances bds_state *states = calloc(2*params->d - 1, sizeof(bds_state)); - treehash_inst treehash[(2*params->d - 1) * (params->tree_height - params->bds_k)]; + treehash_inst *treehash = calloc((2*params->d - 1) * (params->tree_height - params->bds_k), sizeof(treehash_inst)); for (i = 0; i < 2*params->d - 1; i++) { states[i].stack = NULL; states[i].stackoffset = 0; @@ -850,15 +876,15 @@ int xmssmt_core_sign(const xmss_params *params, /* Check if we can still sign with this sk. * If not, return -2 - * - * If this is the last possible signature (because the max index value - * is reached), production implementations should delete the secret key + * + * If this is the last possible signature (because the max index value + * is reached), production implementations should delete the secret key * to prevent accidental further use. - * - * For the case of total tree height of 64 we do not use the last signature - * to be on the safe side (there is no index value left to indicate that the + * + * For the case of total tree height of 64 we do not use the last signature + * to be on the safe side (there is no index value left to indicate that the * key is finished, hence external handling would be necessary) - */ + */ if (idx >= ((1ULL << params->full_height) - 1)) { // Delete secret key here. We only do this in memory, production code // has to make sure that this happens on disk. @@ -870,7 +896,7 @@ int xmssmt_core_sign(const xmss_params *params, goto cleanup; } } - + memcpy(sk_seed, sk+params->index_bytes, params->n); memcpy(sk_prf, sk+params->index_bytes+params->n, params->n); memcpy(pub_seed, sk+params->index_bytes+3*params->n, params->n); @@ -1012,10 +1038,11 @@ int xmssmt_core_sign(const xmss_params *params, } xmssmt_serialize_state(params, sk, states); - goto cleanup; cleanup: + OQS_MEM_insecure_free(treehash); OQS_MEM_insecure_free(states); + OQS_MEM_insecure_free(tmp); return ret; }