Skip to content

Commit

Permalink
Increase robustness against UB.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
roconnor-blockstream committed Jul 5, 2019
1 parent fa33017 commit 8fe63e5
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/scalar_low_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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) {
if (flag && bit < 32)
*r += (1 << bit);
*r += ((uint32_t)1 << bit);
#ifdef VERIFY
VERIFY_CHECK(secp256k1_scalar_check_overflow(r) == 0);
#endif
Expand Down

0 comments on commit 8fe63e5

Please sign in to comment.