ecc: reject compressed EC points with incorrect length#10591
Open
MarkAtwood wants to merge 1 commit into
Open
Conversation
wc_ecc_import_point_der_ex accepts a single byte 0x02 or 0x03 as a valid compressed EC point. It treats the missing X coordinate as zero, decompresses it (producing a valid on-curve point), and wc_ecc_check_key passes. This allows ECDH key agreement with a crafted 1-byte peer public key. Add length validation for compressed points: after identifying 0x02/0x03 format byte, verify that inLen == ecc_sets[curve_idx].size + 1 using unsigned comparison to avoid underflow. Only set compressed = 1 after the length check passes, keeping state consistent on the error path. Reproducer: call EC_POINT_oct2point with a 1-byte buffer containing 0x02 for any NIST curve. Before this fix it succeeds; after, it returns ECC_BAD_ARG_E.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens ECC point import by rejecting compressed EC point encodings whose byte length doesn’t match the expected SEC1/X9.63 compressed form length for the selected curve, preventing truncated inputs such as a bare 0x02/0x03 format byte from being treated as valid points.
Changes:
- Add an exact-length check for compressed points in
wc_ecc_import_point_der_ex()before enabling the compressed decode path. - Ensure
compressedstate is only set after the length check passes, returningECC_BAD_ARG_Eon mismatch.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9633
to
+9638
| if (inLen == (word32)ecc_sets[curve_idx].size + 1) { | ||
| compressed = 1; | ||
| } | ||
| else { | ||
| err = ECC_BAD_ARG_E; | ||
| } |
Comment on lines
+9631
to
+9638
| /* Compressed point must be exactly 1 + field_element_size bytes. | ||
| * Reject truncated inputs (e.g. a bare 0x02/0x03 byte). */ | ||
| if (inLen == (word32)ecc_sets[curve_idx].size + 1) { | ||
| compressed = 1; | ||
| } | ||
| else { | ||
| err = ECC_BAD_ARG_E; | ||
| } |
Comment on lines
+9631
to
+9638
| /* Compressed point must be exactly 1 + field_element_size bytes. | ||
| * Reject truncated inputs (e.g. a bare 0x02/0x03 byte). */ | ||
| if (inLen == (word32)ecc_sets[curve_idx].size + 1) { | ||
| compressed = 1; | ||
| } | ||
| else { | ||
| err = ECC_BAD_ARG_E; | ||
| } |
Contributor
|
Jenkins retest this please |
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
wc_ecc_import_point_der_exaccepts a single byte0x02or0x03as a valid compressed EC point. It treats the missing X coordinate as zero, decompresses it (producing a valid on-curve point), andwc_ecc_check_keypasses. This allows ECDH key agreement with a crafted 1-byte peer public key.Fix
After identifying
0x02/0x03format byte, verify thatinLen == ecc_sets[curve_idx].size + 1using unsigned comparison to avoid underflow. Only setcompressed = 1after the length check passes, keeping state consistent on the error path.Reproducer
Call
EC_POINT_oct2pointwith a 1-byte buffer containing0x02for any NIST curve. Before this fix it succeeds; after, it returnsECC_BAD_ARG_E.Test Plan
0x02input returnsECC_BAD_ARG_E