status-im/secp256k1
2
0
Fork 0
mirror of https://github.com/status-im/secp256k1.git synced 2025-02-24 03:38:12 +00:00
Code Issues Projects Releases Wiki Activity

Merge #647: Increase robustness against UB in secp256k1_scalar_cadd_bit

Browse Source 
0d82732 Improve VERIFY_CHECK of overflow in secp256k1_scalar_cadd_bit. This added check ensures that any curve order overflow doesn't go undetected due a uint32_t overflow. (Russell O'Connor)
8fe63e5 Increase robustness against UB. Thanks to elichai2 who noted that the literal '1' is a signed integer, and that shifting a signed 32-bit integer by 31 bits causes an overflow and yields undefined behaviour. While 'scalar_low_impl''s 'secp256k1_scalar_cadd_bit' is only used for testing purposes and currently the 'bit' parameter is only 0 or 1, it is better to avoid undefined behaviour in case the used domain of 'secp256k1_scalar_cadd_bit' expands. (roconnor-blockstream)

Pull request description:

  Avoid possible, but unlikely undefined behaviour in `scalar_low_impl`'s `secp256k1_scalar_cadd_bit`.
  Thanks to elichai2 who noted that the literal `1` is a signed integer, and that shifting a signed 32-bit integer by 31 bits causes an overflow and yields undefined behaviour.

  Using the unsigned literal `1u` addresses the issue.

ACKs for commit 0d8273:
  real-or-random:
    ACK 0d82732a9a16cecc445e61c718ce9bdc2d228e76
  jonasnick:
    ACK 0d82732a9a16cecc445e61c718ce9bdc2d228e76

Tree-SHA512: 905be3b8b00aa5cc9bd6dabb543745119da8f34181d37765071f28abbc1d6ff3659e3f195b72c2f2d003006678823919668bc0d169ac8b8d4bcc5da671813c99
This commit is contained in:
Tim Ruffing 2019-10-28 11:53:46 +01:00
commit 137d304a6b
No known key found for this signature in database
GPG Key ID: 8C461CCD293F6011
1 changed files with 4 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/scalar_low_impl.h
View File

@ -38,8 +38,11 @@ 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) {
if (flag && bit < 32)
*r += (1 << bit);
*r += ((uint32_t)1 << bit);
#ifdef VERIFY
VERIFY_CHECK(bit < 32);
/* Verify that adding (1 << bit) will not overflow any in-range scalar *r by overflowing the underlying uint32_t. */
VERIFY_CHECK(((uint32_t)1 << bit) - 1 <= UINT32_MAX - EXHAUSTIVE_TEST_ORDER);
VERIFY_CHECK(secp256k1_scalar_check_overflow(r) == 0);
#endif
}