Skip to content

key-wallet: Seed is Copy + not ZeroizeOnDrop — the master seed is never scrubbed #843

Open

Description

@shumkov

Summary

key_wallet::Seed holds the 512-bit BIP32 master seed, but it is never scrubbed
from memory and can be silently duplicated:

// key-wallet/src/seed.rs
#[derive(Clone, Copy, PartialEq, Eq, Hash, Zeroize)]
pub struct Seed([u8; 64]);
  • It derives Copy, so every Seed — and every implicit copy of one — leaves
    an un-scrubbed 64-byte master secret on the stack/heap.
  • Because it is Copy, it cannot be ZeroizeOnDrop (Rust forbids Copy + Drop).
    So the derived Zeroize is never invoked automatically — the bytes linger until
    the memory happens to be overwritten.

This is the master secret from which every wallet key derives, so it's worth
holding to the usual secret-hygiene bar: zeroize-on-drop, and no silent copies.

Proposed fix

  • Drop Copy from Seed; keep Clone (make duplication explicit).
  • Add zeroize::ZeroizeOnDrop so the bytes are scrubbed when a Seed drops.
  • Audit the sites that relied on implicit Copy and make copies explicit
    (.clone()) or pass &Seed — there are ~47 Seed references in key-wallet
    alone, plus downstream consumers.
  • With Seed no longer Copy, from_seed_bytes (and friends) can take
    &[u8; 64], so a caller holding the seed in a Zeroizing buffer avoids an extra
    un-scrubbed by-value copy at the call boundary.

Blast radius / breaking change

Removing Copy from Seed is a breaking API change. Seed is used in ~47
places across key-wallet and by downstream consumers (dashpay/platform's
rs-platform-wallet, key-wallet-manager, key-wallet-ffi); every site relying
on implicit copy needs review. This warrants its own scoped PR (breaking title,
e.g. refactor(key-wallet)!:) plus a coordinated downstream rev bump.

Context

Surfaced from dashpay/platform#3841: create_wallet_from_seed_bytes wraps the seed
in Zeroizing but is forced to make a by-value copy to call from_seed_bytes. The
narrow attempt (#842) only removed that one call-boundary copy and did not
address the underlying Copy/no-scrub issue, so it was closed in favor of this
issue.

🤖 Filed with Claude Code

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions