diff --git a/wolfcrypt/src/wc_lms.c b/wolfcrypt/src/wc_lms.c index 6ddb9164ec..97e9ab029b 100644 --- a/wolfcrypt/src/wc_lms.c +++ b/wolfcrypt/src/wc_lms.c @@ -1210,11 +1210,10 @@ int wc_LmsKey_MakeKey(LmsKey* key, WC_RNG* rng) if (key->priv_data == NULL) { ret = MEMORY_E; } - #ifdef WOLFSSL_WC_LMS_SERIALIZE_STATE else { + /* Loading reads the state before it is computed over. */ XMEMSET(key->priv_data, 0, priv_data_len); } - #endif } if (ret == 0) { WC_DECLARE_VAR(state, LmsState, 1, 0); @@ -1334,6 +1333,10 @@ int wc_LmsKey_Reload(LmsKey* key) if (key->priv_data == NULL) { ret = MEMORY_E; } + else { + /* Loading reads the state before it is computed over. */ + XMEMSET(key->priv_data, 0, priv_data_len); + } } if (ret == 0) { int rv; diff --git a/wolfcrypt/src/wc_lms_impl.c b/wolfcrypt/src/wc_lms_impl.c index 15f7ddf843..3a2133902f 100644 --- a/wolfcrypt/src/wc_lms_impl.c +++ b/wolfcrypt/src/wc_lms_impl.c @@ -1787,8 +1787,10 @@ static int wc_lmots_sign(LmsState* state, const byte* seed, const byte* msg, * @param [in] params LMS parameters. * @param [out] state Private key state. * @param [in] priv_data Private key data. + * @return 0 on success. + * @return BUFFER_E when a stored index is out of range for the parameters. */ -static void wc_lms_priv_state_load(const LmsParams* params, LmsPrivState* state, +static int wc_lms_priv_state_load(const LmsParams* params, LmsPrivState* state, byte* priv_data) { /* Authentication path data. */ @@ -1812,6 +1814,21 @@ static void wc_lms_priv_state_load(const LmsParams* params, LmsPrivState* state, priv_data += 4; ato32(priv_data, &state->leaf.offset); /* priv_data += 4; */ + + /* Stack offset is a byte count into a stack of height + 1 nodes. + * leaf.idx is deliberately wrapped when the cache is empty - don't + * bound it. */ + if ((state->stack.offset > + LMS_STACK_CACHE_LEN(params->height, params->hash_len)) || + ((state->stack.offset % params->hash_len) != 0)) { + return BUFFER_E; + } + /* Leaf cache is a ring of 2^cacheBits nodes. */ + if (state->leaf.offset >= ((word32)1U << params->cacheBits)) { + return BUFFER_E; + } + + return 0; } /* Store the LMS private state into data. @@ -2321,6 +2338,7 @@ static int wc_lms_treehash_update(LmsState* state, LmsPrivState* privState, byte* temp = left + params->hash_len; WC_DECLARE_VAR(stack, byte, (LMS_MAX_HEIGHT + 1) * LMS_MAX_NODE_LEN, 0); byte* sp; + byte* spEnd; word32 max_cb = (word32)1 << params->cacheBits; word32 i; @@ -2338,6 +2356,7 @@ static int wc_lms_treehash_update(LmsState* state, LmsPrivState* privState, XMEMCPY(stack, stackCache->stack, (word32)params->height * params->hash_len); sp = stack + stackCache->offset; + spEnd = stack + LMS_STACK_CACHE_LEN(params->height, params->hash_len); } /* Compute all nodes requested. */ @@ -2393,6 +2412,11 @@ static int wc_lms_treehash_update(LmsState* state, LmsPrivState* privState, j >>= 1; h++; + /* Node to combine with must be on the stack. */ + if ((size_t)(sp - stack) < params->hash_len) { + ret = BUFFER_E; + break; + } sp -= params->hash_len; if (useRoot && (h > params->height - params->rootLevels) && (h <= params->height)) { @@ -2426,6 +2450,10 @@ static int wc_lms_treehash_update(LmsState* state, LmsPrivState* privState, params->hash_len); } } + if ((ret == 0) && ((size_t)(spEnd - sp) < params->hash_len)) { + /* No room on the stack to push onto. */ + ret = BUFFER_E; + } if (ret == 0) { /* Push temp onto the data stack. */ XMEMCPY(sp, temp, params->hash_len); @@ -3486,10 +3514,13 @@ static int wc_hss_presign(LmsState* state, HssPrivKey* priv_key) * @param [in] params LMS parameters. * @param [in, out] key HSS private key. * @param [in] priv_data Private key data. + * @return 0 on success. + * @return BUFFER_E when a stored index is out of range for the parameters. */ -static void wc_hss_priv_data_load(const LmsParams* params, HssPrivKey* key, +static int wc_hss_priv_data_load(const LmsParams* params, HssPrivKey* key, byte* priv_data) { + int ret = 0; #ifndef WOLFSSL_WC_LMS_SMALL int l; #endif @@ -3500,8 +3531,13 @@ static void wc_hss_priv_data_load(const LmsParams* params, HssPrivKey* key, #ifndef WOLFSSL_WC_LMS_SMALL for (l = 0; l < params->levels; l++) { - /* Caches for subtree. */ - wc_lms_priv_state_load(params, &key->state[l], priv_data); + /* Caches for subtree. Keep mapping the rest of the data even on a bad + * state so every pointer is set; the first error is returned. */ + int rc = wc_lms_priv_state_load(params, &key->state[l], priv_data); + + if (ret == 0) { + ret = rc; + } priv_data += LMS_PRIV_STATE_LEN(params->height, params->rootLevels, params->cacheBits, params->hash_len); } @@ -3512,7 +3548,11 @@ static void wc_hss_priv_data_load(const LmsParams* params, HssPrivKey* key, priv_data += LMS_PRIV_KEY_LEN(params->levels, params->hash_len); for (l = 0; l < params->levels - 1; l++) { /* Next subtree's caches. */ - wc_lms_priv_state_load(params, &key->next_state[l], priv_data); + int rc = wc_lms_priv_state_load(params, &key->next_state[l], priv_data); + + if (ret == 0) { + ret = rc; + } priv_data += LMS_PRIV_STATE_LEN(params->height, params->rootLevels, params->cacheBits, params->hash_len); } @@ -3523,6 +3563,8 @@ static void wc_hss_priv_data_load(const LmsParams* params, HssPrivKey* key, key->y = priv_data; #endif /* WOLFSSL_LMS_NO_SIG_CACHE */ #endif /* WOLFSSL_WC_LMS_SMALL */ + + return ret; } #ifndef WOLFSSL_WC_LMS_SMALL @@ -3571,6 +3613,8 @@ static void wc_hss_priv_data_store(const LmsParams* params, HssPrivKey* key, * @param [out] priv_data Private key data. * @param [out] pub_root Public key root node. * @return 0 on success. + * @return BAD_FUNC_ARG when the parameters would make a shift undefined. + * @return BUFFER_E when the stored state has an index out of range. */ int wc_hss_reload_key(LmsState* state, const byte* priv_raw, HssPrivKey* priv_key, byte* priv_data, byte* pub_root) @@ -3589,7 +3633,10 @@ int wc_hss_reload_key(LmsState* state, const byte* priv_raw, } #endif - wc_hss_priv_data_load(state->params, priv_key, priv_data); + /* Not returned on error here: only the no-root branch below uses the state + * as loaded. The others recompute it, over values that may be + * uninitialized. */ + ret = wc_hss_priv_data_load(state->params, priv_key, priv_data); #ifdef WOLFSSL_WC_LMS_SERIALIZE_STATE if (pub_root != NULL) diff --git a/wolfcrypt/src/wc_xmss_impl.c b/wolfcrypt/src/wc_xmss_impl.c index 7cd27a90e4..0ff9c9ea95 100644 --- a/wolfcrypt/src/wc_xmss_impl.c +++ b/wolfcrypt/src/wc_xmss_impl.c @@ -2543,7 +2543,8 @@ typedef struct BdsState { byte* treeHashNode; /* Hashes of nodes to retain - based on K parameter. */ byte* retain; - /* Next leaf to calculate - max 20 bits. */ + /* Next leaf to calculate - max 20 bits. Equals 2^sub_h when the subtree + * has been completed. */ word32 next; /* Current offset into stack - 0... */ word8 offset; @@ -2702,6 +2703,8 @@ static int wc_xmss_bds_state_load(const XmssState* state, byte* sk, const word8 k = params->bds_k; const word32 retainLen = XMSS_RETAIN_LEN(k, n); int i; + int j; + word16 used; /* Skip past standard SK = idx || wots_sk || SK_PRF || root || SEED; */ sk += params->idx_len + 4 * n; @@ -2730,6 +2733,32 @@ static int wc_xmss_bds_state_load(const XmssState* state, byte* sk, sk += 3; bds[i].offset = sk[0]; sk += 1; + + /* Stack holds at most hs + 1 nodes. */ + if (bds[i].offset > (word8)(hs + 1)) { + return WC_FAILURE; + } + /* next counts leaves done rather than indexing one: it is only used + * as an index while below 2^hs and comes to rest at 2^hs when the + * subtree is complete. */ + if (bds[i].next > ((word32)1U << hs)) { + return WC_FAILURE; + } + /* An update pops one stack node per node a tree hash uses. Tree hash + * j completes at height j, so it holds at most j nodes, and all of + * them are on the stack. */ + used = 0; + for (j = 0; j < (int)hsk; j++) { + word8 tu = (word8)(bds[i].treeHash[j * 4 + 3] & 0x7f); + + if (tu > j) { + return WC_FAILURE; + } + used = (word16)(used + tu); + } + if (used > bds[i].offset) { + return WC_FAILURE; + } } if (wots_sigs != NULL) { @@ -2992,10 +3021,17 @@ static void wc_xmss_bds_treehash_update(XmssState* state, BdsState* bds, const word8 n = params->n; HashAddress addrLocal; TreeHash treeHash[1]; - byte* sp = bds->stack + bds->offset * n; + byte* sp; byte* node = state->stack + WC_XMSS_MAX_STACK_LEN - n; word8 h; + /* Stack holds at most sub_h + 1 nodes. */ + if (bds->offset > (word8)(params->sub_h + 1)) { + state->ret = WC_FAILURE; + return; + } + sp = bds->stack + bds->offset * n; + /* Get the tree hash data. */ wc_xmss_bds_state_treehash_get(bds, height, treeHash); /* Copy hash address into local as OTS type. */ @@ -3013,7 +3049,8 @@ static void wc_xmss_bds_treehash_update(XmssState* state, BdsState* bds, h = 0; /* Top node on Stack has same height t' as node. */ - while ((treeHash->used > 0) && (h == bds->height[bds->offset - 1])) { + while ((treeHash->used > 0) && (bds->offset > 0) && + (h == bds->height[bds->offset - 1])) { sp -= n; /* Copy from stack to before last calculated node. */ node -= n; @@ -3036,6 +3073,11 @@ static void wc_xmss_bds_treehash_update(XmssState* state, BdsState* bds, XMEMCPY(bds->treeHashNode + height * n, node, n); treeHash->completed = 1; } + else if (bds->offset > params->sub_h) { + /* No slot left on the stack to push onto. */ + state->ret = WC_FAILURE; + return; + } else { /* Push calculated node onto stack. */ XMEMCPY(sp, node, n); @@ -3141,14 +3183,15 @@ static void wc_xmss_bds_update(XmssState* state, BdsState* bds, { if (bds->next < ((word32)1U << state->params->sub_h)) { const XmssParams* params = state->params; - byte* sp = bds->stack + bds->offset * params->n; + byte* sp; HashAddress addrCopy; XMSS_ADDR_OTS_SET_SUBTREE(addrCopy, addr); - if (bds->height == NULL) { + if ((bds->height == NULL) || (bds->offset > params->sub_h)) { state->ret = WC_FAILURE; return; } + sp = bds->stack + bds->offset * params->n; wc_xmss_bds_next_idx(state, bds, sk_seed, pk_seed, addrCopy, bds->next, bds->height, &bds->offset, &sp); bds->offset++; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 0556fd5351..d5c8e65b31 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -61443,6 +61443,9 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t xmss_test(void) #endif int ret2 = -1; int ret = WC_TEST_RET_ENC_NC; +#ifndef WOLFSSL_NO_MALLOC + byte * sk_snapshot = NULL; +#endif WOLFSSL_ENTER("xmss_test"); #ifndef HAVE_FIPS @@ -61551,6 +61554,17 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t xmss_test(void) ret = wc_XmssKey_Verify(&verifyKey, sig, sigSz, (byte *) msg, msgSz); if (ret != 0) { ERROR_OUT(WC_TEST_RET_ENC_I(i), out); } +#ifndef WOLFSSL_NO_MALLOC + /* Keep an early state - it still has a tree hash in progress, which + * is what the traversal counters below drive. */ + if ((i == 2) && (sk_snapshot == NULL)) { + sk_snapshot = (byte *)XMALLOC(skSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER); + if (sk_snapshot == NULL) { ERROR_OUT(WC_TEST_RET_ENC_ERRNO, out); } + XMEMCPY(sk_snapshot, sk, skSz); + } +#endif + /* Flip bits in a few places throughout the signature, stepping in multiple * of hash size. These should all fail with -1. */ for (j = 0; j < (int) sigSz; j+= 4 * 32) { @@ -61568,10 +61582,67 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t xmss_test(void) } } +#ifndef WOLFSSL_NO_MALLOC + /* The BDS traversal counters sit at the end of the persisted private key + * and are re-parsed on every sign. Corrupt them and the library must come + * back with an error, having stayed inside the key's own buffers. */ + if (sk_snapshot != NULL) { + XmssKey reloadKey; + + /* One extra pass with the state left alone, to show that a good state + * is still accepted. */ + for (j = (int)skSz - 16; j <= (int)skSz; j++) { + XMEMCPY(sk, sk_snapshot, skSz); + if (j < (int)skSz) { + sk[j] = 0xc8; + } + + ret = wc_XmssKey_Init(&reloadKey, NULL, devId); + if (ret == 0) { + ret = wc_XmssKey_SetParamStr(&reloadKey, param); + } + if (ret == 0) { + ret = wc_XmssKey_SetWriteCb(&reloadKey, xmss_write_key_mem); + } + if (ret == 0) { + ret = wc_XmssKey_SetReadCb(&reloadKey, xmss_read_key_mem); + } + if (ret == 0) { + ret = wc_XmssKey_SetContext(&reloadKey, (void *) sk); + } + if (ret == 0) { + ret = wc_XmssKey_Reload(&reloadKey); + } + if (ret == 0) { + sigSz = bufSz; + ret = wc_XmssKey_Sign(&reloadKey, sig, &sigSz, (byte *) msg, + msgSz); + } + if ((ret == 0) && (j == (int)skSz)) { + /* Untouched state - the signature must be the real one. */ + ret = wc_XmssKey_Verify(&verifyKey, sig, sigSz, (byte *) msg, + msgSz); + } + wc_XmssKey_Free(&reloadKey); + + /* A corrupt state may sign or may fail; an untouched one may not + * fail. Under a sanitizer this is also what catches a write that + * left the key's buffers. */ + if ((ret != 0) && (j == (int)skSz)) { + ERROR_OUT(WC_TEST_RET_ENC_I(j), out); + } + } + ret = 0; + } +#endif /* !WOLFSSL_NO_MALLOC */ + out: /* Cleanup everything. */ #ifndef WOLFSSL_NO_MALLOC + XFREE(sk_snapshot, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + sk_snapshot = NULL; + XFREE(sig, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); sig = NULL;