From 45e4fbf2072407c9afceb95367be21c51539419e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Feb 2020 19:08:51 +0000 Subject: [PATCH] Fix handling of large RHS in mp_add_integer_into. While looking over the code for other reasons, I happened to notice that the internal function mp_add_masked_integer_into was using a totally wrong condition to check whether it was about to do an out-of-range right shift: it was comparing a shift count measured in bits against BIGNUM_INT_BYTES. The resulting bug hasn't shown up in the code so far, which I assume is just because no caller is passing any RHS to mp_add_integer_into bigger than about 1 or 2. And it doesn't show up in the test suite because I hadn't tested those functions. Now I am testing them, and the newly added test fails when built for 16-bit BignumInt if you back out the actual fix in this commit. (cherry picked from commit 921118dbea743cc1ba78e21b01924343a077064f) --- mpint.c | 5 +++-- test/cryptsuite.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/mpint.c b/mpint.c index 8140fafe..3d7a3987 100644 --- a/mpint.c +++ b/mpint.c @@ -758,7 +758,7 @@ static BignumCarry mp_add_masked_integer_into( for (size_t i = 0; i < rw; i++) { BignumInt aword = mp_word(a, i); size_t shift = i * BIGNUM_INT_BITS; - BignumInt bword = shift < BIGNUM_INT_BYTES ? b >> shift : 0; + BignumInt bword = shift < CHAR_BIT*sizeof(b) ? b >> shift : 0; BignumInt out; bword = (bword ^ b_xor) & b_and; BignumADC(out, carry, aword, bword, carry); @@ -799,7 +799,8 @@ static void mp_add_integer_into_shifted_by_words( * shift n down. If it's 0, we add zero bits into r, and * leave n alone. */ BignumInt bword = n & -(BignumInt)indicator; - uintmax_t new_n = (BIGNUM_INT_BITS < 64 ? n >> BIGNUM_INT_BITS : 0); + uintmax_t new_n = (BIGNUM_INT_BYTES < sizeof(n) ? + n >> BIGNUM_INT_BITS : 0); n ^= (n ^ new_n) & -(uintmax_t)indicator; BignumInt aword = mp_word(a, i); diff --git a/test/cryptsuite.py b/test/cryptsuite.py index f76c3ed2..995110b0 100755 --- a/test/cryptsuite.py +++ b/test/cryptsuite.py @@ -335,6 +335,22 @@ class mpint(MyTestBase): bm = mp_copy(bi) self.assertEqual(int(mp_mul(am, bm)), ai * bi) + def testAddInteger(self): + initial = mp_copy(4444444444444444444444444) + + x = mp_new(mp_max_bits(initial) + 64) + + # mp_{add,sub}_integer_into should be able to cope with any + # uintmax_t. Test a number that requires more than 32 bits. + mp_add_integer_into(x, initial, 123123123123123) + self.assertEqual(int(x), 4444444444567567567567567) + mp_sub_integer_into(x, initial, 123123123123123) + self.assertEqual(int(x), 4444444444321321321321321) + + # mp_mul_integer_into only takes a uint16_t integer input + mp_mul_integer_into(x, initial, 10001) + self.assertEqual(int(x), 44448888888888888888888884444) + def testDivision(self): divisors = [1, 2, 3, 2**16+1, 2**32-1, 2**32+1, 2**128-159, 141421356237309504880168872420969807856967187537694807]