Skip to content

Commit

Permalink
Possible UB in secp256k1_scalar_cadd_bit
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
roconnor-blockstream committed Jul 3, 2019
1 parent fa33017 commit 6499ebe
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/scalar_low_impl.h
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 += (1u << bit);
#ifdef VERIFY
VERIFY_CHECK(secp256k1_scalar_check_overflow(r) == 0);
#endif
Expand Down

0 comments on commit 6499ebe

Please sign in to comment.