fix(public-key): enforce message length validation for ECDSA signatures#4693
Open
sergei-boiko-trustwallet wants to merge 13 commits intomasterfrom
Open
fix(public-key): enforce message length validation for ECDSA signatures#4693sergei-boiko-trustwallet wants to merge 13 commits intomasterfrom
sergei-boiko-trustwallet wants to merge 13 commits intomasterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens cryptographic input validation by enforcing fixed digest/message sizes for ECDSA-family verification and signing, aiming to prevent misuse and improve consistency across key types.
Changes:
- Added an
ecdsaMessageSizeconstant (32 bytes) to standardize expected ECDSA digest length. - Introduced
validateMessageLength()and integrated it intoPublicKey::verify()to reject non-conforming message sizes for ECDSA-like key types while allowing ED25519 variants. - Hardened
ecdsa_sign_digest_checked()to require an exact 32-byte digest for ECDSA signing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/PublicKey.h | Adds a shared constant for expected ECDSA message/digest size and fixes a comment typo. |
| src/PublicKey.cpp | Makes signature-length validation file-local and adds message-length validation to PublicKey::verify(). |
| src/PrivateKey.cpp | Enforces exact digest length in the checked ECDSA signing path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Binary size comparison➡️ aarch64-apple-ios: 14.34 MB ➡️ aarch64-apple-ios-sim: 14.34 MB ➡️ aarch64-linux-android: 18.77 MB ➡️ armv7-linux-androideabi: 16.20 MB ➡️ wasm32-unknown-emscripten: 13.68 MB |
* Fix `PrivateKey::signAsDER`
… for long digest signing
…ix/public-key-verify-message
…llet/wallet-core into fix/buffer-over-read
Fix buffer over-read
Add signature size verification
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.
This pull request introduces improved validation for message lengths in ECDSA signature verification, ensuring stricter checks and consistency across cryptographic operations. The main changes include adding message length validation for relevant public key types and updating the ECDSA signing function to enforce exact digest size requirements.
Validation improvements:
validateMessageLengthinsrc/PublicKey.cppto check that messages have the correct length for ECDSA-based public key types, while allowing flexible sizes for ED25519 variants.validateMessageLengthinto thePublicKey::verifymethod so that verification fails if the message length is incorrect for the specified key type.ECDSA signing enforcement:
ecdsa_sign_digest_checkedfunction insrc/PrivateKey.cppto require the digest size to be exactly 32 bytes, improving the robustness of signature generation.ecdsaMessageSizeinPublicKey.hto define the expected message length for ECDSA operations, supporting consistent validation.Code quality:
validateSignatureLengthto be a static function insrc/PublicKey.cpp, clarifying its intended usage and scope.