Repository navigation
Conversation
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The LTC path permits a stack-buffer overflow with oversized moduli, and scalar-handling regressions and P-256 test-configuration failures remain unresolved.
5 open issues
What changed in this PR
This PR hardens ECC scalar multiplication for F-16901 by rejecting negative scalars in wolfCrypt and adding scalar reduction in the OpenSSL compatibility layer.
Changes:
- Adds scalar validation across software and hardware implementations, plus coordinate bounds checks for NXP LTC.
- Reduces negative and oversized
EC_POINT_mul()scalars modulo order × cofactor. - Adds regression tests and documents the range error.
| File | Description |
|---|---|
wolfcrypt/src/port/st/stm32.c |
Validates scalars and sets the result’s y sign positive. |
wolfcrypt/src/port/nxp/ksdk_port.c |
Adds scalar and coordinate checks; sets y positive. |
wolfcrypt/src/ecc.c |
Rejects negative scalars and validates fixed-point arguments before allocation. |
tests/api/test_ossl_ec.h |
Registers the compatibility-layer regression test. |
tests/api/test_ossl_ec.c |
Tests negative and oversized scalar reduction. |
tests/api/test_ecc.h |
Registers the wolfCrypt scalar-validation test. |
tests/api/test_ecc.c |
Tests scalar rejection and positive-scalar acceptance. |
src/pk_ec.c |
Adds scalar reduction before ECC multiplication. |
doc/dox_comments/header_files/ecc.h |
Documents ECC_OUT_OF_RANGE_E for invalid scalars. |
Review effort: Balanced
Member
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12036
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfcrypt-port-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): doc/dox_comments/header_files/ecc.h, tests/api/test_ecc.h, tests/api/test_ossl_ec.h
Coverage: 6 of 6 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
Frauschi
force-pushed
the
ecc-negative-scalar
branch
2 times, most recently
from
November 6, 2026 11:27
1cf7741 to
b49acb7
Compare
wc_ecc_mulmod_ex(), wc_ecc_mulmod_ex2() and ecc_mul2add() checked the scalar only by bit length and then read its magnitude, so a negative k computed |k|*G and returned success. That is the negation of the right result. Negative values reach these functions with fastmath, heap math and sp_int with WOLFSSL_SP_INT_NEGATIVE, which every OPENSSL_EXTRA build defines. Return ECC_OUT_OF_RANGE_E for a negative scalar at every entry point, as ecc_make_pub_sw() and the signature range checks already do, including the NXP LTC and STM32 PKA ports, which have their own. Those ports set the sign of the result's y from k; they now set it positive, since reading the coordinates leaves the sign of a reused point unchanged. The fixed-point ecc_mul2add() also gains the argument checks the other implementation has, done before it allocates. The SP-only wc_ecc_mulmod_ex2() and the fixed-point SP-only wc_ecc_mulmod_ex() and wc_ecc_mulmod_ex2() also check the scalar's length, as the SP-only wc_ecc_mulmod_ex() does. The SP code drops the high digits of a longer scalar and returned a wrong point. The LTC port checks the length as the generic code does, rejects point coordinates wider than the prime with IS_POINT_E, and refuses a prime wider than its coordinate buffers, since it copied all of them into fixed stack buffers. The STM32 PKA port checks the length too, so it returns ECC_OUT_OF_RANGE_E rather than BUFFER_E. EC_POINT_mul() passed the BIGNUM's mp_int straight through. OpenSSL reduces a negative or over-wide scalar modulo the number of points on the curve, the order times the cofactor. Reduce n and m the same way when they are negative or wider than the prime, which some backends reject, so they give the same result. Some math libraries leave the remainder with the sign of the dividend, so a negative remainder gets the modulus added. Reducing by the order alone would be wrong for a point outside the prime-order subgroup on a curve with a cofactor. The bound comes from the curve's degree, so scalars in range cost nothing extra, and the modulus is computed once. Fixes F-16901.
Frauschi
force-pushed
the
ecc-negative-scalar
branch
from
November 6, 2026 14:17
b49acb7 to
bef27f1
Compare
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.
Description
ECC scalar multiplication computed |k|·G for a negative scalar and returned success, reachable from every OPENSSL_EXTRA build. Every entry point now rejects it, and
EC_POINT_mul()reduces such scalars as OpenSSL does.Fixes F-16901.
wc_ecc_mulmod_ex(),wc_ecc_mulmod_ex2()andecc_mul2add()returnECC_OUT_OF_RANGE_Efor a negative scalar, in the generic, SP and fixed-point codekand the point coordinates it copies into fixed stack buffersEC_POINT_mul()reduces a negative or over-wide scalar modulo order × cofactorecc_mul2add()checks its arguments before allocatingLimitations
EC_KEYprivate key still reaches the SP ECDH and sign paths as |k|; that is a separate follow-upTesting
test_wc_ecc_mulmod_negative: negative and oversized scalars forwc_ecc_mulmod(),_ex,_ex2andecc_mul2add()test_wolfSSL_EC_POINT_mul_negative: -5, -order and order·2^64+5 give the same points as their reduced values