From be78ab95084ed22bac386865f74d7ebbe622feeb Mon Sep 17 00:00:00 2001 From: czurnieden Date: Tue, 9 May 2023 17:17:12 +0200 Subject: [PATCH 1/2] Fix possible integer overflow (cherry picked from commit beba892bc0d4e4ded4d667ab1d2a94f4d75109a9) --- mp_2expt.c | 4 ++++ mp_grow.c | 4 ++++ mp_init_size.c | 4 ++++ s_mp_mul.c | 4 ++++ s_mp_mul_comba.c | 4 ++++ s_mp_mul_high.c | 4 ++++ s_mp_mul_high_comba.c | 4 ++++ 7 files changed, 28 insertions(+) diff --git a/mp_2expt.c b/mp_2expt.c index 4a5fc0063..c11fe4cb3 100644 --- a/mp_2expt.c +++ b/mp_2expt.c @@ -12,6 +12,10 @@ mp_err mp_2expt(mp_int *a, int b) { mp_err err; + if (b < 0) { + return MP_VAL; + } + /* zero a as per default */ mp_zero(a); diff --git a/mp_grow.c b/mp_grow.c index 5bca1b5ff..551d1a3bc 100644 --- a/mp_grow.c +++ b/mp_grow.c @@ -6,6 +6,10 @@ /* grow as required */ mp_err mp_grow(mp_int *a, int size) { + if (size < 0) { + return MP_VAL; + } + /* if the alloc size is smaller alloc more ram */ if (a->alloc < size) { mp_digit *dp; diff --git a/mp_init_size.c b/mp_init_size.c index e28a3cd49..87a4256e8 100644 --- a/mp_init_size.c +++ b/mp_init_size.c @@ -6,6 +6,10 @@ /* init an mp_init for a given size */ mp_err mp_init_size(mp_int *a, int size) { + if (size < 0) { + return MP_VAL; + } + size = MP_MAX(MP_MIN_DIGIT_COUNT, size); if (size > MP_MAX_DIGIT_COUNT) { diff --git a/s_mp_mul.c b/s_mp_mul.c index fb99d8054..3394c142e 100644 --- a/s_mp_mul.c +++ b/s_mp_mul.c @@ -13,6 +13,10 @@ mp_err s_mp_mul(const mp_int *a, const mp_int *b, mp_int *c, int digs) mp_err err; int pa, ix; + if (digs < 0) { + return MP_VAL; + } + /* can we use the fast multiplier? */ if ((digs < MP_WARRAY) && (MP_MIN(a->used, b->used) < MP_MAX_COMBA)) { diff --git a/s_mp_mul_comba.c b/s_mp_mul_comba.c index 1afa1fc68..ca89ff9dd 100644 --- a/s_mp_mul_comba.c +++ b/s_mp_mul_comba.c @@ -26,6 +26,10 @@ mp_err s_mp_mul_comba(const mp_int *a, const mp_int *b, mp_int *c, int digs) mp_digit W[MP_WARRAY]; mp_word _W; + if (digs < 0) { + return MP_VAL; + } + /* grow the destination as required */ if ((err = mp_grow(c, digs)) != MP_OKAY) { return err; diff --git a/s_mp_mul_high.c b/s_mp_mul_high.c index 1bde00aa9..fd532ebd8 100644 --- a/s_mp_mul_high.c +++ b/s_mp_mul_high.c @@ -12,6 +12,10 @@ mp_err s_mp_mul_high(const mp_int *a, const mp_int *b, mp_int *c, int digs) int pa, pb, ix; mp_err err; + if (digs < 0) { + return MP_VAL; + } + /* can we use the fast multiplier? */ if (MP_HAS(S_MP_MUL_HIGH_COMBA) && ((a->used + b->used + 1) < MP_WARRAY) diff --git a/s_mp_mul_high_comba.c b/s_mp_mul_high_comba.c index 74960aca6..b5ac06d74 100644 --- a/s_mp_mul_high_comba.c +++ b/s_mp_mul_high_comba.c @@ -19,6 +19,10 @@ mp_err s_mp_mul_high_comba(const mp_int *a, const mp_int *b, mp_int *c, int digs mp_digit W[MP_WARRAY]; mp_word _W; + if (digs < 0) { + return MP_VAL; + } + /* grow the destination as required */ pa = a->used + b->used; if ((err = mp_grow(c, pa)) != MP_OKAY) { From 6175cca524822b2b7fd4fa263ef6ee76be5cd77a Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 5 Sep 2023 07:33:46 +0200 Subject: [PATCH 2/2] Prevent potential runtime-error in tests `abs()` can only convert `INT_MIN-1 .. -1` to a positive `int`. Nothing prevents the PRNG to create `INT_MIN` which then leads to a failure of the call to `abs()` as seen in [0]. Instead add an unsigned version of the function reading from the PRNG, so we also don't need to make an absolute value from it. [0] https://github.com/libtom/libtommath/pull/555#issuecomment-1572004663 ``` demo/test.c:1112:13: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself ``` Signed-off-by: Steffen Jaeckel --- demo/test.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/demo/test.c b/demo/test.c index f86853644..c5141ef7f 100644 --- a/demo/test.c +++ b/demo/test.c @@ -14,9 +14,9 @@ static long rand_long(void) return x; } -static int rand_int(void) +static unsigned int rand_uint(void) { - int x; + unsigned int x; if (s_mp_rand_jenkins(&x, sizeof(x)) != MP_OKAY) { fprintf(stderr, "s_mp_rand_source failed\n"); exit(EXIT_FAILURE); @@ -24,6 +24,11 @@ static int rand_int(void) return x; } +static int rand_int(void) +{ + return (int)rand_uint(); +} + static int32_t rand_int32(void) { int32_t x; @@ -448,7 +453,7 @@ static int test_mp_signed_rsh(void) l = rand_long(); mp_set_l(&a, l); - em = abs(rand_int()) % 32; + em = rand_uint() % 32; mp_set_l(&d, l >> em); @@ -1091,7 +1096,8 @@ static int test_mp_prime_next_prime(void) static int test_mp_montgomery_reduce(void) { mp_digit mp; - int ix, i, n; + int ix, n; + unsigned int i; char buf[4096]; /* size_t written; */ @@ -1106,7 +1112,7 @@ static int test_mp_montgomery_reduce(void) printf(" digit size: %2d\r", i); fflush(stdout); for (n = 0; n < 1000; n++) { - DO(mp_rand(&a, i)); + DO(mp_rand(&a, (int)i)); a.dp[0] |= 1; /* let's see if R is right */ @@ -1115,7 +1121,7 @@ static int test_mp_montgomery_reduce(void) /* now test a random reduction */ for (ix = 0; ix < 100; ix++) { - DO(mp_rand(&c, 1 + (abs(rand_int()) % (2*i)))); + DO(mp_rand(&c, 1 + (int)(rand_uint() % (2*i)))); DO(mp_copy(&c, &d)); DO(mp_copy(&c, &e)); @@ -1274,7 +1280,7 @@ static int test_s_mp_div_3(void) printf("\r %9d", cnt); fflush(stdout); } - DO(mp_rand(&a, (abs(rand_int()) % 128) + 1)); + DO(mp_rand(&a, (int)(rand_uint() % 128) + 1)); DO(mp_div(&a, &d, &b, &e)); DO(s_mp_div_3(&a, &c, &r2));