Skip to content

fix: zero-init ML-DSA keygen DMA response#447

Open
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/mldsa-keygen-dma-uninit-res
Open

fix: zero-init ML-DSA keygen DMA response#447
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/mldsa-keygen-dma-uninit-res

Conversation

@MarkAtwood

Copy link
Copy Markdown

Bug

In _HandleMlDsaKeyGenDma (src/wh_server_crypto.c), the response struct is declared uninitialized:

whMessageCrypto_MlDsaKeyGenDmaResponse res;

Multiple error paths return before res is written — _IsMlDsaLevelSupported failure (WH_ERROR_BADARGS), wc_MlDsaKey_Init/SetParams/MakeKey failure, and DMA WRITE_PRE address-check failure before res.keyId/res.keySize are set. All fall through to wh_MessageCrypto_TranslateMlDsaKeyGenDmaResponse with *outSize = sizeof(res), transmitting indeterminate stack bytes for res.keyId, res.keySize, and res.dmaAddrStatus to the client. (dmaAddrStatus.badAddr is only set on WH_ERROR_ACCESS, so it is garbage for every other error.)

Fix

Zero-initialize the response struct at declaration:

whMessageCrypto_MlDsaKeyGenDmaResponse res = {0};

Error paths now transmit deterministic zeros instead of stack garbage. This matches the sibling DMA handlers, e.g. _HandleAesCtrDma which does memset(&res, 0, sizeof(res)) before any response send.

Build verification

Compiled the translation unit against the standard wolfHSM server + ML-DSA + DMA config on Ubuntu 24.04:

gcc -c -DWOLFHSM_CFG -DWOLFHSM_CFG_ENABLE_SERVER -DWOLFHSM_CFG_DMA -DWOLFSSL_HAVE_MLDSA \
    -I. -Itest/config -I<wolfssl-install>/include src/wh_server_crypto.c

Exit 0, _HandleMlDsaKeyGenDma present as a defined text symbol.

Reported by static analysis (Fenrir finding 463).

Copilot AI review requested due to automatic review settings July 9, 2026 23:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes an information-leak bug in the ML-DSA key generation DMA handler by ensuring the DMA response struct is deterministically initialized on all error paths.

Changes:

  • Zero-initialize whMessageCrypto_MlDsaKeyGenDmaResponse res in _HandleMlDsaKeyGenDma to prevent transmitting uninitialized stack bytes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants