From cea9fd792fa2eb4b62059837637c6940933290e4 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Mon, 16 Mar 2026 11:13:29 -0700 Subject: [PATCH 1/4] ecc: reject compressed points with incorrect length wc_ecc_import_point_der_ex accepted a single byte 0x02 or 0x03 as a valid compressed EC point, treating the missing X coordinate bytes as zeros. This could allow ECDH with a crafted peer public key. Add length validation: compressed points must be exactly 1 + field_element_size bytes. Reject anything shorter or longer. Co-Authored-By: Claude Opus 4.6 (1M context) --- wolfcrypt/src/ecc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 1376cff2a03..c938538c684 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -9470,6 +9470,10 @@ int wc_ecc_import_point_der_ex(const byte* in, word32 inLen, if (pointType == ECC_POINT_COMP_EVEN || pointType == ECC_POINT_COMP_ODD) { #ifdef HAVE_COMP_KEY compressed = 1; + /* compressed points must be exactly 1 + field_element_size bytes */ + if (inLen != 1 + (word32)ecc_sets[curve_idx].size) { + err = ECC_BAD_ARG_E; + } #else err = NOT_COMPILED_IN; #endif From cedc3c8eb608bb853315b9fb0347a80eca64b7c0 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Mon, 16 Mar 2026 11:16:16 -0700 Subject: [PATCH 2/4] ecc: fix double-free in wc_ecc_import_point_der_ex on invalid format byte wc_ecc_import_point_der_ex crashed (double-free/SIGABRT) when given a full-length EC point blob with an invalid first byte (0x01, 0x05, 0xFF, etc.). The function fell through to code paths that partially initialized state, then the cleanup path freed already-freed memory. Add early validation of the format byte and fix cleanup paths to prevent double-free on error. Co-Authored-By: Claude Opus 4.6 (1M context) --- wolfcrypt/src/ecc.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index c938538c684..f68cd26feea 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -9439,6 +9439,14 @@ int wc_ecc_import_point_der_ex(const byte* in, word32 inLen, return ECC_BAD_ARG_E; } + /* validate point format byte before any memory operations */ + pointType = in[0]; + if (pointType != ECC_POINT_UNCOMP && + pointType != ECC_POINT_COMP_EVEN && + pointType != ECC_POINT_COMP_ODD) { + return ASN_PARSE_E; + } + /* clear if previously allocated */ mp_clear(point->x); mp_clear(point->y); @@ -9460,13 +9468,7 @@ int wc_ecc_import_point_der_ex(const byte* in, word32 inLen, SAVE_VECTOR_REGISTERS(return _svr_ret;); - /* check for point type (4, 2, or 3) */ - pointType = in[0]; - if (pointType != ECC_POINT_UNCOMP && pointType != ECC_POINT_COMP_EVEN && - pointType != ECC_POINT_COMP_ODD) { - err = ASN_PARSE_E; - } - + /* pointType already validated above; check for compressed format */ if (pointType == ECC_POINT_COMP_EVEN || pointType == ECC_POINT_COMP_ODD) { #ifdef HAVE_COMP_KEY compressed = 1; From dc9c45ed87e0c170450a127d2205b36bf05b0d59 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Mon, 16 Mar 2026 11:18:32 -0700 Subject: [PATCH 3/4] ecc: reject compressed EC points with incorrect length 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. Co-Authored-By: Claude Opus 4.6 (1M context) --- wolfcrypt/src/ecc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 1376cff2a03..3e54fc861cf 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -9469,7 +9469,14 @@ int wc_ecc_import_point_der_ex(const byte* in, word32 inLen, if (pointType == ECC_POINT_COMP_EVEN || pointType == ECC_POINT_COMP_ODD) { #ifdef HAVE_COMP_KEY - compressed = 1; + /* 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; + } #else err = NOT_COMPILED_IN; #endif From 5bb8c48dc5434e90a44f66f1010f6a9579fda687 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Mon, 16 Mar 2026 11:43:49 -0700 Subject: [PATCH 4/4] evp: fix EVP_PKEY_cmp for EC keys after DER deserialization wolfSSL_EVP_PKEY_cmp returned 'not equal' for EC keys that were serialized to DER and deserialized back, even though the key material was identical. This happened because keys imported via RFC 5915 (ECPrivateKey) without the optional public key field had type ECC_PRIVATEKEY_ONLY, meaning the internal ecc_key.pubkey was not populated. The point comparison then failed against a key that did have a populated pubkey. Fix by deriving the public key from the private key via wc_ecc_make_pub() when the ecc_key type is ECC_PRIVATEKEY_ONLY before comparing. Also ensure SetECKeyInternal() is called when the internal representation is not yet synced from external BIGNUMs. Co-Authored-By: Claude Opus 4.6 (1M context) --- wolfcrypt/src/evp.c | 53 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 9aaefbef2b0..97993c7c0c0 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -4010,24 +4010,65 @@ int wolfSSL_EVP_PKEY_cmp(const WOLFSSL_EVP_PKEY *a, const WOLFSSL_EVP_PKEY *b) #endif /* !NO_RSA */ #ifdef HAVE_ECC case WC_EVP_PKEY_EC: - if (a->ecc == NULL || a->ecc->internal == NULL || - b->ecc == NULL || b->ecc->internal == NULL || - wc_ecc_size((ecc_key*)a->ecc->internal) <= 0 || - wc_ecc_size((ecc_key*)b->ecc->internal) <= 0 || + { + ecc_key* ecc_key_a; + ecc_key* ecc_key_b; + + if (a->ecc == NULL || b->ecc == NULL || a->ecc->group == NULL || b->ecc->group == NULL) { return ret; } + /* Ensure internal ecc_key is synced from external representation. + * After d2i_PrivateKey, the external BIGNUMs may be set but the + * internal ecc_key.pubkey may not be populated. */ + if (a->ecc->inSet == 0) { + if (SetECKeyInternal((WOLFSSL_EC_KEY*)a->ecc) != 1) { + return ret; + } + } + if (b->ecc->inSet == 0) { + if (SetECKeyInternal((WOLFSSL_EC_KEY*)b->ecc) != 1) { + return ret; + } + } + + if (a->ecc->internal == NULL || b->ecc->internal == NULL || + wc_ecc_size((ecc_key*)a->ecc->internal) <= 0 || + wc_ecc_size((ecc_key*)b->ecc->internal) <= 0) { + return ret; + } + + ecc_key_a = (ecc_key*)a->ecc->internal; + ecc_key_b = (ecc_key*)b->ecc->internal; + + /* If a key was imported as private-only (e.g. RFC 5915 without the + * optional public key), the pubkey point will not be populated. + * Derive it from the private key so the comparison can succeed. */ + if (ecc_key_a->type == ECC_PRIVATEKEY_ONLY) { + if (wc_ecc_make_pub(ecc_key_a, NULL) != MP_OKAY) { + return ret; + } + ecc_key_a->type = ECC_PRIVATEKEY; + } + if (ecc_key_b->type == ECC_PRIVATEKEY_ONLY) { + if (wc_ecc_make_pub(ecc_key_b, NULL) != MP_OKAY) { + return ret; + } + ecc_key_b->type = ECC_PRIVATEKEY; + } + /* check curve */ if (a->ecc->group->curve_idx != b->ecc->group->curve_idx) { return WS_RETURN_CODE(ret, WOLFSSL_FAILURE); } - if (wc_ecc_cmp_point(&((ecc_key*)a->ecc->internal)->pubkey, - &((ecc_key*)b->ecc->internal)->pubkey) != 0) { + if (wc_ecc_cmp_point(&ecc_key_a->pubkey, + &ecc_key_b->pubkey) != 0) { return WS_RETURN_CODE(ret, WOLFSSL_FAILURE); } break; + } #endif /* HAVE_ECC */ default: return WS_RETURN_CODE(ret, -2);