Make secp256k1_scalar_get_bits support 32-bit reads

The old code would trigger UB when count=32.
This commit is contained in:
Pieter Wuille
2021-12-28 18:13:04 -05:00
parent 5005abee60
commit e03dcc44b5
4 changed files with 15 additions and 11 deletions

View File

@@ -22,10 +22,10 @@
/** Clear a scalar to prevent the leak of sensitive data. */ /** Clear a scalar to prevent the leak of sensitive data. */
static void secp256k1_scalar_clear(secp256k1_scalar *r); static void secp256k1_scalar_clear(secp256k1_scalar *r);
/** Access bits from a scalar. All requested bits must belong to the same 32-bit limb. */ /** Access bits (1 < count <= 32) from a scalar. All requested bits must belong to the same 32-bit limb. */
static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count); static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count);
/** Access bits from a scalar. Not constant time in offset and count. */ /** Access bits (1 < count <= 32) from a scalar. offset + count must be < 256. Not constant time in offset and count. */
static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count); static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count);
/** Set a scalar from a big endian byte array. The scalar will be reduced modulo group order `n`. /** Set a scalar from a big endian byte array. The scalar will be reduced modulo group order `n`.

View File

@@ -47,21 +47,22 @@ 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_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
VERIFY_CHECK((offset + count - 1) >> 6 == offset >> 6); VERIFY_CHECK((offset + count - 1) >> 6 == offset >> 6);
return (a->d[offset >> 6] >> (offset & 0x3F)) & ((((uint64_t)1) << count) - 1); return (a->d[offset >> 6] >> (offset & 0x3F)) & (0xFFFFFFFF >> (32 - count));
} }
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) { SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count < 32); VERIFY_CHECK(count > 0 && count <= 32);
VERIFY_CHECK(offset + count <= 256); VERIFY_CHECK(offset + count <= 256);
if ((offset + count - 1) >> 6 == offset >> 6) { if ((offset + count - 1) >> 6 == offset >> 6) {
return secp256k1_scalar_get_bits_limb32(a, offset, count); return secp256k1_scalar_get_bits_limb32(a, offset, count);
} else { } else {
VERIFY_CHECK((offset >> 6) + 1 < 4); VERIFY_CHECK((offset >> 6) + 1 < 4);
return ((a->d[offset >> 6] >> (offset & 0x3F)) | (a->d[(offset >> 6) + 1] << (64 - (offset & 0x3F)))) & ((((uint64_t)1) << count) - 1); return ((a->d[offset >> 6] >> (offset & 0x3F)) | (a->d[(offset >> 6) + 1] << (64 - (offset & 0x3F)))) & (0xFFFFFFFF >> (32 - count));
} }
} }

View File

@@ -64,21 +64,22 @@ 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_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count > 0 && count <= 32);
VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5); VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5);
return (a->d[offset >> 5] >> (offset & 0x1F)) & ((1 << count) - 1); return (a->d[offset >> 5] >> (offset & 0x1F)) & (0xFFFFFFFF >> (32 - count));
} }
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) { SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);
VERIFY_CHECK(count < 32); VERIFY_CHECK(count > 0 && count <= 32);
VERIFY_CHECK(offset + count <= 256); VERIFY_CHECK(offset + count <= 256);
if ((offset + count - 1) >> 5 == offset >> 5) { if ((offset + count - 1) >> 5 == offset >> 5) {
return secp256k1_scalar_get_bits_limb32(a, offset, count); return secp256k1_scalar_get_bits_limb32(a, offset, count);
} else { } else {
VERIFY_CHECK((offset >> 5) + 1 < 8); VERIFY_CHECK((offset >> 5) + 1 < 8);
return ((a->d[offset >> 5] >> (offset & 0x1F)) | (a->d[(offset >> 5) + 1] << (32 - (offset & 0x1F)))) & ((((uint32_t)1) << count) - 1); return ((a->d[offset >> 5] >> (offset & 0x1F)) | (a->d[(offset >> 5) + 1] << (32 - (offset & 0x1F)))) & (0xFFFFFFFF >> (32 - count));
} }
} }

View File

@@ -30,11 +30,13 @@ 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_INLINE static uint32_t secp256k1_scalar_get_bits_limb32(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);
if (offset < 32) VERIFY_CHECK(count > 0 && count <= 32);
return ((*a >> offset) & ((((uint32_t)1) << count) - 1)); if (offset < 32) {
else return (*a >> offset) & (0xFFFFFFFF >> (32 - count));
} else {
return 0; return 0;
} }
}
SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) { SECP256K1_INLINE static uint32_t secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
SECP256K1_SCALAR_VERIFY(a); SECP256K1_SCALAR_VERIFY(a);