← Watch feed
Add VERIFY_CHECKs that flags are 0 or 1
Public commit record
78/100 · Adequate
What the developer wrote
Authored by John Moffett
Add VERIFY_CHECKs that flags are 0 or 1
Flags for constant-time masking rely
on the values being exactly 0 or 1 rather
than 0 or true. Add VERIFY_CHECKs to enforce
in VERIFY builds as a preventative
measure and add documentation where relevant.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit adds safety checks and documentation to ensure that 'flag' values used in secret-handling code are exactly 0 or 1, not just any 'true' value. These flags control constant-time selection and memory wiping. If a caller passed a different non-zero value, the masking math could behave incorrectly and potentially leak secret information through timing or produce wrong results. The new checks only fire in special VERIFY builds, so they are a defensive hardening measure rather than a fix for an active bug.
Recommended action
Treat as a defensive hardening commit. Review all callers of the modified cmov/cadd_bit/cond_negate/memczero functions to confirm they already pass only 0 or 1. Consider whether any production path could pass a truthy non-1 flag; if so, a runtime fix is needed beyond these VERIFY checks. Otherwise, include in normal release testing and note as hardening in release notes.
Security signals we found
01
constant-time masking invariant enforcement
02
VERIFY_CHECK hardening for secret-dependent flag values
03
documentation update clarifying 0/1 flag contract
04
potential timing-side-channel risk if non-{0,1} flag reaches production
Evidence from the diff
The patch inserts VERIFY_CHECK(flag == 0 || flag == 1) into cmov, cadd_bit, cond_negate, and memczero implementations across field, group, scalar, and utility code, plus updates header comments. These functions compute masks such as -flag or ~flag+~0, which yield the expected all-0/all-1 mask only when flag is exactly 0 or 1. Other truthy values would create partial masks, breaking constant-time guarantees or causing incorrect conditional operations. VERIFY_CHECK is compiled out in production, so this is a development/test-time invariant enforcement and documentation improvement.
Changed components
src/field.hsrc/field_10x26_impl.hsrc/field_5x52_impl.hsrc/group.hsrc/group_impl.hsrc/modules/musig/session_impl.hsrc/scalar.hsrc/scalar_4x64_impl.hsrc/scalar_8x32_impl.hsrc/scalar_low_impl.hsrc/util.hInspect captured patch +30 / −8
diff --git a/src/field.h b/src/field.h
index 1f6ba74..e3f0234 100644
--- a/src/field.h
+++ b/src/field.h
@@ -307,7 +307,8 @@ static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe
*/
static void secp256k1_fe_from_storage(secp256k1_fe *r, const secp256k1_fe_storage *a);
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/
+/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time.
+ * Both *r and *a must be initialized. Flag must be 0 or 1. */
static void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag);
/** Conditionally move a field element in constant time.
diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h
index ea14c27..aa45434 100644
--- a/src/field_10x26_impl.h
+++ b/src/field_10x26_impl.h
@@ -1014,6 +1014,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25
SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
uint32_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
@@ -1097,6 +1098,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) {
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
uint32_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h
index 46dca6b..3a97613 100644
--- a/src/field_5x52_impl.h
+++ b/src/field_5x52_impl.h
@@ -349,6 +349,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25
SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
uint64_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = vflag + ~((uint64_t)0);
mask1 = ~mask0;
@@ -416,6 +417,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) {
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
uint64_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = vflag + ~((uint64_t)0);
mask1 = ~mask0;
diff --git a/src/group.h b/src/group.h
index 05ae0d2..ee3ebbb 100644
--- a/src/group.h
+++ b/src/group.h
@@ -169,10 +169,12 @@ static void secp256k1_ge_to_storage(secp256k1_ge_storage *r, const secp256k1_ge
/** Convert a group element back from the storage type. */
static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storage *a);
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/
+/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time.
+ * Both *r and *a must be initialized. Flag must be 0 or 1. */
static void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag);
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/
+/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time.
+ * Both *r and *a must be initialized. Flag must be 0 or 1. */
static void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag);
/** Rescale a jacobian point by b which must be non-zero. Constant-time. */
diff --git a/src/group_impl.h b/src/group_impl.h
index 81a24b9..f516965 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -898,6 +898,7 @@ static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storag
static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag) {
SECP256K1_GEJ_VERIFY(r);
SECP256K1_GEJ_VERIFY(a);
+ VERIFY_CHECK(flag == 0 || flag == 1);
secp256k1_fe_cmov(&r->x, &a->x, flag);
secp256k1_fe_cmov(&r->y, &a->y, flag);
@@ -908,6 +909,7 @@ static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k
}
static SECP256K1_INLINE void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag) {
+ VERIFY_CHECK(flag == 0 || flag == 1);
secp256k1_fe_storage_cmov(&r->x, &a->x, flag);
secp256k1_fe_storage_cmov(&r->y, &a->y, flag);
}
diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h
index 6a80a27..6fb3288 100644
--- a/src/modules/musig/session_impl.h
+++ b/src/modules/musig/session_impl.h
@@ -69,7 +69,8 @@ static int secp256k1_musig_secnonce_load(const secp256k1_context* ctx, secp256k1
return 1;
}
-/* If flag is true, invalidate the secnonce; otherwise leave it. Constant-time. */
+/* If flag is 1, invalidate the secnonce; if flag is 0, leave it.
+ * Constant-time. Flag must be 0 or 1. */
static void secp256k1_musig_secnonce_invalidate(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, int flag) {
secp256k1_memczero(secnonce->data, sizeof(secnonce->data), flag);
/* The flag argument is usually classified. So, the line above makes the
diff --git a/src/scalar.h b/src/scalar.h
index 70f49b1..40d6719 100644
--- a/src/scalar.h
+++ b/src/scalar.h
@@ -48,7 +48,7 @@ static void secp256k1_scalar_get_b32(unsigned char *bin, const secp256k1_scalar*
/** Add two scalars together (modulo the group order). Returns whether it overflowed. */
static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b);
-/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. */
+/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. Flag must be 0 or 1. */
static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag);
/** Multiply two scalars (modulo the group order). */
@@ -78,7 +78,7 @@ static int secp256k1_scalar_is_even(const secp256k1_scalar *a);
/** Check whether a scalar is higher than the group order divided by 2. */
static int secp256k1_scalar_is_high(const secp256k1_scalar *a);
-/** Conditionally negate a number, in constant time.
+/** Conditionally negate a number, in constant time. Flag must be 0 or 1.
* Returns -1 if the number was negated, 1 otherwise */
static int secp256k1_scalar_cond_negate(secp256k1_scalar *a, int flag);
@@ -95,7 +95,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT
/** Multiply a and b (without taking the modulus!), divide by 2**shift, and round to the nearest integer. Shift must be at least 256. */
static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b, unsigned int shift);
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/
+/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. Both *r and *a must be initialized. Flag must be 0 or 1. */
static void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag);
/** Check invariants on a scalar (no-op unless VERIFY is enabled). */
diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h
index 807b9b7..a5bf18f 100644
--- a/src/scalar_4x64_impl.h
+++ b/src/scalar_4x64_impl.h
@@ -120,6 +120,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a,
static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) {
secp256k1_uint128 t;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(r);
VERIFY_CHECK(bit < 256);
@@ -259,6 +260,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
uint64_t mask = -vflag;
uint64_t nonzero = (secp256k1_scalar_is_zero(r) != 0) - 1;
secp256k1_uint128 t;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(r);
secp256k1_u128_from_u64(&t, r->d[0] ^ mask);
@@ -911,6 +913,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
uint64_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(a);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h
index 2610496..aa87b1d 100644
--- a/src/scalar_8x32_impl.h
+++ b/src/scalar_8x32_impl.h
@@ -147,6 +147,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a,
static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) {
uint64_t t;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(r);
VERIFY_CHECK(bit < 256);
@@ -314,6 +315,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
uint32_t mask = -vflag;
uint32_t nonzero = 0xFFFFFFFFUL * (secp256k1_scalar_is_zero(r) == 0);
uint64_t t = (uint64_t)(r->d[0] ^ mask) + ((SECP256K1_N_0 + 1) & mask);
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(r);
r->d[0] = t & nonzero; t >>= 32;
@@ -709,6 +711,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
uint32_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(a);
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
diff --git a/src/scalar_low_impl.h b/src/scalar_low_impl.h
index 84e1a38..628bfd3 100644
--- a/src/scalar_low_impl.h
+++ b/src/scalar_low_impl.h
@@ -56,6 +56,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a,
static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) {
SECP256K1_SCALAR_VERIFY(r);
+ VERIFY_CHECK(flag == 0 || flag == 1);
if (flag && bit < 32)
*r += ((uint32_t)1 << bit);
@@ -121,6 +122,7 @@ static int secp256k1_scalar_is_high(const secp256k1_scalar *a) {
static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
SECP256K1_SCALAR_VERIFY(r);
+ VERIFY_CHECK(flag == 0 || flag == 1);
if (flag) secp256k1_scalar_negate(r, r);
@@ -157,6 +159,7 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
uint32_t mask0, mask1;
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
SECP256K1_SCALAR_VERIFY(a);
SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r));
diff --git a/src/util.h b/src/util.h
index 94e0837..46ab381 100644
--- a/src/util.h
+++ b/src/util.h
@@ -212,6 +212,7 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
take only be 0 or 1, which leads to variable time code. */
volatile int vflag = flag;
unsigned char mask = -(unsigned char) vflag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
while (len) {
*p &= ~mask;
p++;
@@ -294,7 +295,8 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size
return ret;
}
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized and non-negative.*/
+/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time.
+ * Both *r and *a must be initialized and non-negative. Flag must be 0 or 1. */
static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) {
unsigned int mask0, mask1, r_masked, a_masked;
/* Access flag with a volatile-qualified lvalue.
@@ -302,6 +304,7 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag)
take only be 0 or 1, which leads to variable time code. */
volatile int vflag = flag;
+ VERIFY_CHECK(flag == 0 || flag == 1);
/* Casting a negative int to unsigned and back to int is implementation defined behavior */
VERIFY_CHECK(*r >= 0 && *a >= 0);
Risk score
Our methodology →Why this scored 36/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.