Conversation
…ed s-normalization When pubkey is available, verify recovered private key via d*G == pubkey and try both s polarities to handle mixed BIP62 low-s/high-s signatures. Without pubkey, behavior is unchanged (backward compatible).
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/attack/nonce_reuse.rs">
<violation number="1" location="src/attack/nonce_reuse.rs:94">
P2: Pubkey verification can falsely reject valid recovered keys because `0x` prefixes are not normalized before comparison.</violation>
</file>
Architecture diagram
sequenceDiagram
participant NRA as NonceReuseAttack
participant Math as math.rs (Recover logic)
participant K256 as k256 (Elliptic Curve)
Note over NRA,K256: try_recover_pair flow
alt Pubkey is Present
loop NEW: For s2 and -s2 (BIP62 Normalization)
NRA->>Math: recover_nonce(z1, z2, s1, current_s2)
Math-->>NRA: k (nonce)
NRA->>Math: recover_private_key(r1, s1, z1, k)
Math-->>NRA: d (private key)
NRA->>K256: NEW: verify_key_matches_pubkey(d, expected_pubkey)
K256->>K256: Calculate ProjectivePoint (d * G)
K256->>K256: Convert to AffinePoint
K256-->>NRA: True if hex(compressed/uncompressed) matches
opt Match found
Note over NRA: Return verified RecoveredKey
end
end
else Pubkey is Missing (Legacy Path)
NRA->>Math: recover_nonce(z1, z2, s1, s2)
Math-->>NRA: k (nonce)
NRA->>Math: recover_private_key(r1, s1, z1, k)
Math-->>NRA: d (private key)
Note over NRA: Return unverified RecoveredKey
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
oritwoen
approved these changes
Mar 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
d*G == pubkey) instead of returning unverified resultsProblem
The nonce-reuse attack previously returned the first mathematically valid private key without verifying it against the known public key. This caused two issues:
swas BIP62-canonicalized (low-s) and the other wasn't, the standard formulak = (z₁-z₂)/(s₁-s₂)yields the wrong nonce, and recovery silently failsSolution
When pubkey is present:
d*Gand compare against the provided pubkey (compressed and uncompressed)s₂and-s₂, verifying each candidate — only return the key that matchesbiased_nonce::verify_candidateandpolynonce::verify_keyWhen pubkey is absent:
Tests
test_nonce_reuse_recovery_with_pubkey_verification— EC-consistent signatures with knownd, verifies correct key recoverytest_nonce_reuse_recovery_negated_s_with_pubkey— Same setup but withs₂negated, proves s-polarity handling works