Skip to content

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

Merged
gilescope merged 5 commits into
mainfrom
fix/PM-22024-default-wallet-seed
Apr 17, 2026
Merged

fix: verify and close PM-22024 default wallet seed audit finding#1109
gilescope merged 5 commits into
mainfrom
fix/PM-22024-default-wallet-seed

Conversation

@m2ux

@m2ux m2ux commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

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
@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
@m2ux m2ux requested a review from gilescope April 17, 2026 12:33
m2ux and others added 5 commits April 17, 2026 13:46
Signed-off-by: Mike Clay <mike.clay@shielded.io>
Signed-off-by: Mike Clay <mike.clay@shielded.io>
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
Signed-off-by: Mike Clay <mike.clay@shielded.io>
Made-with: Cursor
Signed-off-by: Mike Clay <mike.clay@shielded.io>
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux m2ux force-pushed the fix/PM-22024-default-wallet-seed branch from f80a98f to 6946b44 Compare April 17, 2026 12:50
@gilescope gilescope added this pull request to the merge queue Apr 17, 2026
Merged via the queue into main with commit b8b737a Apr 17, 2026
51 of 52 checks passed
@gilescope gilescope deleted the fix/PM-22024-default-wallet-seed branch April 17, 2026 14:57
m2ux added a commit that referenced this pull request Apr 23, 2026
* chore: add change file for PM-22024 wallet seed audit verification


* chore: add PR link to change file


* test: add compile_fail regression test for WalletSeed Default removal

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

* ci: retrigger CI after transient Docker Hub auth failure

Made-with: Cursor

* fix: add GitHub issue reference to changes file


---------


Signed-off-by: Mike Clay <mike.clay@shielded.io>
m2ux added a commit that referenced this pull request Apr 23, 2026
* chore: add change file for PM-22024 wallet seed audit verification


* chore: add PR link to change file


* test: add compile_fail regression test for WalletSeed Default removal

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

* ci: retrigger CI after transient Docker Hub auth failure

Made-with: Cursor

* fix: add GitHub issue reference to changes file


---------


Signed-off-by: Mike Clay <mike.clay@shielded.io>
@gilescope gilescope self-assigned this Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:node Node component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants