Skip to content

Reject NULL metadata in checked NVM add path#462

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
yosuke-wolfssl:fix/f_6789
Open

Reject NULL metadata in checked NVM add path#462
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
yosuke-wolfssl:fix/f_6789

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown

Fix NULL metadata dereference in the DMA NVM add path

Fixes F-6789.

Problem

wh_Nvm_AddObjectChecked dereferenced its meta argument twice before
validating it: once for meta->id when calling wh_Nvm_CheckPolicy, and
again for the sanitized = *meta copy that strips server-only flags.

That pointer is client-controlled on the DMA path. The ADDOBJECTDMA
handler in wh_Server_HandleNvmRequest obtains it from
wh_Server_DmaProcessClientAddress, which defaults the transformed
address to the raw client address and returns it unchanged when no DMA
callback and no DMA allowlist are registered. That permit-all setup is a
supported configuration, and 0 is a valid DMA address, so a client that
sends metadata_hostaddr == 0 crashed the server with a NULL dereference.

The client API does not prevent this either: wh_Client_NvmAddObjectDma
accepts a NULL metadata pointer and forwards a zero metadata_hostaddr.

The other two callers of wh_Nvm_AddObjectChecked pass stack addresses,
so only the DMA path was affected.

Fix

Two layers:

  • wh_Nvm_AddObjectChecked returns WH_ERROR_BADARGS when meta is
    NULL, matching the validation wh_NvmFlash_AddObject and
    wh_NvmFlashLog_AddObject already perform.
  • The ADDOBJECTDMA handler rejects a NULL transformed metadata pointer
    at the boundary. This is defense in depth: it short-circuits before
    taking the NVM lock and before the LMS/XMSS stateful-blob scan, and it
    makes the handler's contract explicit. The check sits after both DMA
    pre-operations so the matching post-operation cleanup still runs on the
    rejection path.

The data pointer is deliberately not checked at the boundary. Both NVM
backends already reject data == NULL with a non-zero length, and
wh_Crypto_IsStatefulSigPrivBlob NULL-checks its buffer, so a boundary
check there could never change the outcome and would be unreachable by
any test.

Testing

_whTest_NvmDmaNullAddrs is added to the existing whTest_NvmDma entry
point in test-refactor/client-server/wh_test_nvm_ops.c. It sends a zero
metadata DMA address and expects WH_ERROR_BADARGS, and also sends a
valid metadata pointer with a zero data address and a non-zero length to
pin the backend rejection. The POSIX test port registers neither a DMA
callback nor an allowlist, so it reproduces the vulnerable permit-all
configuration directly.

Before the fix the suite died with SIGSEGV inside whTest_NvmDma. After:

  • cd test-refactor && make check DMA=1 — 41 passed, 21 skipped, 0 failed
  • cd test-refactor && make check — 41 passed, 21 skipped, 0 failed
  • cd test && make DMA=1 && make run — exit 0

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jul 21, 2026
Copilot AI review requested due to automatic review settings July 21, 2026 02:55

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

This PR hardens the DMA NVM “add object” path against a client-controlled NULL metadata pointer that could previously crash the server, and adds regression coverage for the vulnerable configuration (permit-all DMA with no callback/allowlist).

Changes:

  • Add an early meta == NULL validation in wh_Nvm_AddObjectChecked() to avoid dereferencing a NULL metadata pointer.
  • Add a boundary check in the server’s ADDOBJECTDMA handler to reject a NULL transformed metadata pointer before entering the NVM layer.
  • Extend the DMA NVM refactor test to exercise NULL/zero DMA address behavior and confirm correct error returns.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test-refactor/client-server/wh_test_nvm_ops.c Adds a DMA regression test for NULL metadata DMA address and NULL data-with-length behavior.
src/wh_server_nvm.c Rejects metadata == NULL in the ADDOBJECTDMA handler as defense-in-depth.
src/wh_nvm.c Adds meta == NULL argument validation in wh_Nvm_AddObjectChecked() to prevent NULL dereference.

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

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #462

Scan targets checked: wolfhsm-core-bugs, wolfhsm-src

No new issues found in the changed files. ✅

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.

5 participants