Skip to content

fix: verify and close PM-22024 default wallet seed audit finding#1109

Open
m2ux wants to merge 5 commits intomainfrom
fix/PM-22024-default-wallet-seed
Open

fix: verify and close PM-22024 default wallet seed audit finding#1109
m2ux wants to merge 5 commits intomainfrom
fix/PM-22024-default-wallet-seed

Conversation

@m2ux
Copy link
Copy Markdown
Contributor

@m2ux m2ux commented Mar 27, 2026

Summary

Verify and formally close the Least Authority audit finding (A2 Issue D) regarding the Default implementation on WalletSeed that produced an all-zero seed. The core fix was merged in PR #804; this PR adds a compile-time regression test preventing re-introduction.

🎫 Ticket 📐 Engineering 🧪 Test Plan


Motivation

The Least Authority Node DIFF Audit (Feb 2026) identified that WalletSeed::default() returned Self::Medium([0; 32]) — a deterministic all-zero seed that could lead to predictable wallet key material and recoverable secret keys. This was classified as High severity.

PR #804 removed the Default impl and replaced the single internal placeholder usage with an explicit initializer. Code analysis confirms the fix is complete: the placeholder WalletSeed::Short([0; 16]) in ClaimMintInfo::from_context() is always overwritten by set_rewards() before build() uses it for key derivation. The Least Authority auditor (Mirco) confirmed the fix.


Changes

  • Regression testcompile_fail doctest on WalletSeed verifying .default() does not compile (E0599), preventing re-introduction of the vulnerability. Runs for both ledger_7 and ledger_8 module variants.
  • Change file — Changelog entry documenting the audit verification.

📌 Submission Checklist

  • Changes are backward-compatible (or flagged if breaking)
  • Pull request description explains why the change is needed
  • Self-reviewed the diff
  • I have included a change file, or skipped for this reason: included
  • If the changes introduce a new feature, I have bumped the node minor version
  • Update documentation (if relevant)
  • No new todos introduced

🔱 Fork Strategy

  • Node Runtime Update
  • Node Client Update
  • Other
  • N/A

🗹 TODO before merging

  • Regression test passes (2/2 compile_fail doctests + 16 unit tests)
  • Verification evidence posted to PM-22024
  • Auditor acknowledged fix
  • Ready for review

@m2ux m2ux force-pushed the fix/PM-22024-default-wallet-seed branch from 3b2ef99 to 52266df Compare March 27, 2026 11:54
Audit finding A2-D (Least Authority, Feb 2026): WalletSeed must not
implement Default. The previous impl returned Medium([0; 32]) — an
all-zero seed producing predictable wallet keys. Removed in PR #804.

The compile_fail doctest ensures WalletSeed::default() does not compile,
preventing re-introduction of the vulnerability.

JIRA: https://shielded.atlassian.net/browse/PM-22024
Made-with: Cursor
@m2ux m2ux marked this pull request as ready for review March 27, 2026 15:44
@m2ux m2ux requested a review from a team as a code owner March 27, 2026 15:44
@m2ux m2ux self-assigned this Mar 31, 2026
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.

1 participant