Skip to content

Zeroize local seed buffer in PrivateKey::random#2995

Open
chefsale wants to merge 1 commit into
masterfrom
claude/zeroize-private-seed-identity-fhn6d8
Open

Zeroize local seed buffer in PrivateKey::random#2995
chefsale wants to merge 1 commit into
masterfrom
claude/zeroize-private-seed-identity-fhn6d8

Conversation

@chefsale

Copy link
Copy Markdown
Member

The local [u8;32] secret buffer was filled with key material and moved
into the PrivateKey, but the stack copy was left un-zeroized. Zeroize it
after constructing the key so the seed does not linger in memory.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01W9KHEfpGFsSaJ6XTnHoFoM

The local [u8;32] secret buffer was filled with key material and moved
into the PrivateKey, but the stack copy was left un-zeroized. Zeroize it
after constructing the key so the seed does not linger in memory.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W9KHEfpGFsSaJ6XTnHoFoM
@github-actions

Copy link
Copy Markdown

Your PR title does not adhere to the Conventional Commits convention:

<type>(<scope>): <subject>

Common errors to avoid:

  1. The title must be in lower case.
  2. Allowed type values are: build, ci, docs, feat, fix, perf, refactor, test.

@meroreviewer meroreviewer Bot 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.

🤖 MeroReviewer

Reviewed by 1 agents | Quality score: 85% | Review time: 48.0s


✅ No Issues Found

All agents reviewed the code and found no issues. LGTM! 🎉


🤖 Generated by MeroReviewer | Review ID: review-d69a4b5e

@meroreviewer meroreviewer Bot 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.

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 28% | Review time: 47.3s

💡 1 suggestions. See inline comments.


🤖 Generated by AI Code Reviewer | Review ID: review-fa09069a

// after being moved into the key.
secret.zeroize();

key

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.

💡 Zeroize may be optimized away by the compiler

Calling secret.zeroize() on a stack-local [u8; 32] after it has been moved into Self::from(secret) is correct in intent, but the Rust compiler (or LLVM) may still optimize this call away as a dead store, since secret is no longer used after the zeroize call and the optimizer can see that the bytes are not read again. The zeroize crate's Zeroize impl for primitive arrays uses volatile_set_memory / write_volatile internally to prevent this, so in practice the call is safe — but it is worth confirming that the zeroize dependency version used here provides the volatile-write guarantee (versions ≥ 1.x do). If the zeroize crate's impl is correct, this is fine as-is.

Suggested fix:

No code change needed if `zeroize` >= 1.x is in use (it uses volatile writes). Consider adding a comment referencing this guarantee: `// zeroize uses volatile writes to prevent the compiler from eliding this.`

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.

2 participants