Improve checks for scalar _get_bits methods
What the developer wrote
Authored by Peter.Dettman
Improve checks for scalar _get_bits methods
What changed, and why it matters
This commit tightens internal safety checks in the code that extracts bit chunks from large secret numbers (scalars) used in elliptic-curve cryptography. The changes make the library more defensive against accidental misuse, but the added checks are only active in special test/verification builds, not in normal production builds. There is no direct evidence this fixes an exploitable vulnerability in released software.
Treat as a hardening/defensive improvement rather than an urgent security patch. Review callers of secp256k1_scalar_get_bits_limb32 and secp256k1_scalar_get_bits_var to confirm they already respect the documented 32-bit, in-bounds contract. If running with VERIFY_CHECK enabled, ensure the new assertions do not break existing tests. No CVE or advisory action is indicated by the commit alone.
Security signals we found
Strengthened bounds/overflow-style VERIFY_CHECK preconditions on scalar bit extraction
Changed boundary check from (offset + count - 1) >> 6 == offset >> 6 to >> 5 in 4x64 limb32 path, aligning the no-cross-boundary test with the 32-bit output rather than the 64-bit storage limb
Added offset <= 256 - count precondition to all three implementations
Added non-zero intra-limb offset check for the cross-limb branch in 4x64 get_bits_var
VERIFY_CHECK macros are typically disabled in production builds, limiting runtime effect
Evidence from the diff
The patch modifies secp256k1_scalar_get_bits_limb32 and secp256k1_scalar_get_bits_var across three scalar implementations (4x64, 8x32, and low/test). It replaces or augments VERIFY_CHECK assertions to ensure offset/count stay within the 256-bit scalar and that 32-bit reads do not cross limb boundaries in the limb32 variant. In the 4x64 implementation, the inline fast path now duplicates the mask/shift logic instead of calling the limb32 helper, and adds a check that a cross-limb read only happens when the bit offset within the 64-bit limb is non-zero. These are all VERIFY_CHECK-level changes, meaning they are compiled out in standard non-verify builds.
Changed components
src/scalar_4x64_impl.hsrc/scalar_8x32_impl.hsrc/scalar_low_impl.hInspect captured patch +17 / −6
diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h
index a5bf18f..b914136 100644
--- a/src/scalar_4x64_impl.h
+++ b/src/scalar_4x64_impl.h
@@ -41,7 +41,8 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
- VERIFY_CHECK((offset + count - 1) >> 6 == offset >> 6);
+ VERIFY_CHECK(offset <= 256 - count);
+ VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5);
return (a->d[offset >> 6] >> (offset & 0x3F)) & (0xFFFFFFFF >> (32 - count));
}
@@ -49,12 +50,13 @@ SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
- VERIFY_CHECK(offset + count <= 256);
+ VERIFY_CHECK(offset <= 256 - count);
if ((offset + count - 1) >> 6 == offset >> 6) {
- return secp256k1_scalar_get_bits_limb32(a, offset, count);
+ return (a->d[offset >> 6] >> (offset & 0x3F)) & (0xFFFFFFFF >> (32 - count));
} else {
VERIFY_CHECK((offset >> 6) + 1 < 4);
+ VERIFY_CHECK((offset & 0x3F) > 0);
return ((a->d[offset >> 6] >> (offset & 0x3F)) | (a->d[(offset >> 6) + 1] << (64 - (offset & 0x3F)))) & (0xFFFFFFFF >> (32 - count));
}
}
diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h
index aa87b1d..055918c 100644
--- a/src/scalar_8x32_impl.h
+++ b/src/scalar_8x32_impl.h
@@ -54,6 +54,7 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
+ VERIFY_CHECK(offset <= 256 - count);
VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5);
return (a->d[offset >> 5] >> (offset & 0x1F)) & (0xFFFFFFFF >> (32 - count));
@@ -62,7 +63,7 @@ SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
- VERIFY_CHECK(offset + count <= 256);
+ VERIFY_CHECK(offset <= 256 - count);
if ((offset + count - 1) >> 5 == offset >> 5) {
return secp256k1_scalar_get_bits_limb32(a, offset, count);
diff --git a/src/scalar_low_impl.h b/src/scalar_low_impl.h
index 628bfd3..164cda4 100644
--- a/src/scalar_low_impl.h
+++ b/src/scalar_low_impl.h
@@ -27,8 +27,10 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
-
VERIFY_CHECK(count > 0 && count <= 32);
+ VERIFY_CHECK(offset <= 256 - count);
+ VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5);
+
if (offset < 32) {
return (*a >> offset) & (0xFFFFFFFF >> (32 - count));
} else {
@@ -38,8 +40,14 @@ SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a);
+ VERIFY_CHECK(count > 0 && count <= 32);
+ VERIFY_CHECK(offset <= 256 - count);
- return secp256k1_scalar_get_bits_limb32(a, offset, count);
+ if (offset < 32) {
+ return (*a >> offset) & (0xFFFFFFFF >> (32 - count));
+ } else {
+ return 0;
+ }
}
SECP256K1_INLINE static int secp256k1_scalar_check_overflow(const secp256k1_scalar *a) { return *a >= EXHAUSTIVE_TEST_ORDER; }
Why this scored 26/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.