field: correct fe_equal's b magnitude bound
What the developer wrote
Authored by Lőrinc
field: correct fe_equal's b magnitude bound
`secp256k1_fe_equal` negates `a` before adding `b`.
That gives the temporary value magnitude 2, and the following field addition requires the input magnitudes to sum to at most 32.
So the largest `b` magnitude the implementation can accept is 30, not 31.
Lower the documented and checked bound for `b` to 30.
Adjust the focused test to use random field elements with randomized magnitudes within the accepted `a <= 1` and `b <= 30` bounds.
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Co-authored-by: Tim Ruffing <me@real-or-random.org>
What changed, and why it matters
This commit fixes an off-by-one error in the documented and runtime-checked input limit for a low-level math helper called secp256k1_fe_equal. The function compares two finite-field values for equality. It first negates one input (which temporarily raises its internal 'magnitude' by 1) and then adds the other input. Because the addition can only safely accept inputs whose magnitudes sum to 32, the second input's magnitude must be at most 30, not 31. The previous bound of 31 was therefore one too high and could have allowed an internal overflow in the magnitude bookkeeping. The patch lowers the documented bound and the VERIFY_MAGNITUDE check from 31 to 30, and adds a focused test exercising the corrected boundary. This is a correctness/reliability fix in cryptographic arithmetic, not a directly exploitable remote vulnerability.
Treat as a low-severity correctness fix. Review callers of secp256k1_fe_equal to confirm none relied on passing b with magnitude 31; if any did, they need separate adjustment. Ensure the new test passes under all build configurations, especially those with VERIFY enabled. No urgent deployment is required beyond normal update cadence.
Security signals we found
Off-by-one in cryptographic field-element magnitude bound
Internal invariant violation in secp256k1_fe_equal
Addition-after-negation magnitude accounting error
Defensive fix with new boundary test
Evidence from the diff
secp256k1_fe_equal in src/field_impl.h computes na = -a + b. The negate step produces a temporary with magnitude m(a)+1. The subsequent fe_add requires the sum of input magnitudes to be ≤ 32. With m(a) ≤ 1, the temporary has m(na) ≤ 2, so m(b) must satisfy 2 + m(b) ≤ 32, i.e., m(b) ≤ 30. The old bound of 31 violated this invariant. The patch updates the header comment in src/field.h, the SECP256K1_FE_VERIFY_MAGNITUDE assertion in src/field_impl.h, and adds run_fe_equal_magnitude_boundaries in src/tests.c to test randomized magnitudes up to the new a≤1, b≤30 limits. The fix is local and defensive; it prevents a potential magnitude overflow that could lead to incorrect normalization assumptions in field arithmetic.
Changed components
src/field.hsrc/field_impl.hsrc/tests.csecp256k1_fe_equal functionInspect captured patch +15 / −2
diff --git a/src/field.h b/src/field.h
index 945029e..8b25d99 100644
--- a/src/field.h
+++ b/src/field.h
@@ -166,7 +166,7 @@ static int secp256k1_fe_is_odd(const secp256k1_fe *a);
/** Determine whether two field elements are equal.
*
* On input, a and b must be valid field elements with magnitudes not exceeding
- * 1 and 31, respectively.
+ * 1 and 30, respectively.
* Returns a = b (mod p).
*/
static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b);
diff --git a/src/field_impl.h b/src/field_impl.h
index 7aa7de4..19af6aa 100644
--- a/src/field_impl.h
+++ b/src/field_impl.h
@@ -27,7 +27,7 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp
SECP256K1_FE_VERIFY(a);
SECP256K1_FE_VERIFY(b);
SECP256K1_FE_VERIFY_MAGNITUDE(a, 1);
- SECP256K1_FE_VERIFY_MAGNITUDE(b, 31);
+ SECP256K1_FE_VERIFY_MAGNITUDE(b, 30);
secp256k1_fe_negate(&na, a, 1);
secp256k1_fe_add(&na, b);
diff --git a/src/tests.c b/src/tests.c
index b0d94d6..e821738 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -3064,6 +3064,18 @@ static int fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) {
return secp256k1_fe_equal(&an, &bn);
}
+static void run_fe_equal_magnitude_boundaries(void) {
+ int i;
+ secp256k1_fe a, b;
+ for (i = 0; i < 100 * COUNT; ++i) {
+ testutil_random_fe(&a);
+ b = a;
+ testutil_random_fe_magnitude(&a, 1);
+ testutil_random_fe_magnitude(&b, 30);
+ CHECK(secp256k1_fe_equal(&a, &b));
+ }
+}
+
static void run_field_convert(void) {
static const unsigned char b32[32] = {
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
@@ -7970,6 +7982,7 @@ static const struct tf_test_entry tests_scalar[] = {
static const struct tf_test_entry tests_field[] = {
CASE(field_half),
CASE(field_misc),
+ CASE(fe_equal_magnitude_boundaries),
CASE(field_convert),
CASE(field_be32_overflow),
CASE(fe_mul),
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.