From e28b35b0a39de28fa2f71aa78071d1ad62deaceb Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 8 Jun 2015 19:23:48 +0100 Subject: [PATCH] Improve integer-type hygiene in bignum code. In many places I was using an 'unsigned int', or an implicit int by virtue of writing an undecorated integer literal, where what was really wanted was a BignumInt. In particular, this substitution breaks in any situation where BignumInt is _larger_ than unsigned - which it is shortly about to be. --- sshbn.c | 25 +++++++++++++------------ sshccp.c | 6 ++++-- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/sshbn.c b/sshbn.c index 5a7835ed..fd9e5c0a 100644 --- a/sshbn.c +++ b/sshbn.c @@ -515,7 +515,7 @@ static void monty_reduce(BignumInt *x, const BignumInt *n, } static void internal_add_shifted(BignumInt *number, - unsigned n, int shift) + BignumInt n, int shift) { int word = 1 + (shift / BIGNUM_INT_BITS); int bshift = shift % BIGNUM_INT_BITS; @@ -546,8 +546,7 @@ static void internal_mod(BignumInt *a, int alen, BignumInt *m, int mlen, BignumInt *quot, int qshift) { - BignumInt m0, m1; - unsigned int h; + BignumInt m0, m1, h; int i, k; m0 = m[0]; @@ -559,7 +558,7 @@ static void internal_mod(BignumInt *a, int alen, for (i = 0; i <= alen - mlen; i++) { BignumDblInt t; - unsigned int q, r, c, ai1; + BignumInt q, r, c, ai1; if (i == 0) { h = 0; @@ -614,7 +613,7 @@ static void internal_mod(BignumInt *a, int alen, for (k = mlen - 1; k >= 0; k--) { t = MUL_WORD(q, m[k]); t += c; - c = (unsigned)(t >> BIGNUM_INT_BITS); + c = (BignumInt)(t >> BIGNUM_INT_BITS); if ((BignumInt) t > a[i + k]) c++; a[i + k] -= (BignumInt) t; @@ -697,7 +696,7 @@ Bignum modpow_simple(Bignum base_in, Bignum exp, Bignum mod) /* Skip leading zero bits of exp. */ i = 0; j = BIGNUM_INT_BITS-1; - while (i < (int)exp[0] && (exp[exp[0] - i] & (1 << j)) == 0) { + while (i < (int)exp[0] && (exp[exp[0] - i] & ((BignumInt)1 << j)) == 0) { j--; if (j < 0) { i++; @@ -710,7 +709,7 @@ Bignum modpow_simple(Bignum base_in, Bignum exp, Bignum mod) while (j >= 0) { internal_mul(a + mlen, a + mlen, b, mlen, scratch); internal_mod(b, mlen * 2, m, mlen, NULL, 0); - if ((exp[exp[0] - i] & (1 << j)) != 0) { + if ((exp[exp[0] - i] & ((BignumInt)1 << j)) != 0) { internal_mul(b + mlen, n, a, mlen, scratch); internal_mod(a, mlen * 2, m, mlen, NULL, 0); } else { @@ -847,7 +846,7 @@ Bignum modpow(Bignum base_in, Bignum exp, Bignum mod) /* Skip leading zero bits of exp. */ i = 0; j = BIGNUM_INT_BITS-1; - while (i < (int)exp[0] && (exp[exp[0] - i] & (1 << j)) == 0) { + while (i < (int)exp[0] && (exp[exp[0] - i] & ((BignumInt)1 << j)) == 0) { j--; if (j < 0) { i++; @@ -860,7 +859,7 @@ Bignum modpow(Bignum base_in, Bignum exp, Bignum mod) while (j >= 0) { internal_mul(a + len, a + len, b, len, scratch); monty_reduce(b, n, mninv, scratch, len); - if ((exp[exp[0] - i] & (1 << j)) != 0) { + if ((exp[exp[0] - i] & ((BignumInt)1 << j)) != 0) { internal_mul(b + len, x, a, len, scratch); monty_reduce(a, n, mninv, scratch, len); } else { @@ -1139,7 +1138,8 @@ Bignum bignum_from_bytes(const unsigned char *data, int nbytes) result[i] = 0; for (i = nbytes; i--;) { unsigned char byte = *data++; - result[1 + i / BIGNUM_INT_BYTES] |= byte << (8*i % BIGNUM_INT_BITS); + result[1 + i / BIGNUM_INT_BYTES] |= + (BignumInt)byte << (8*i % BIGNUM_INT_BITS); } while (result[0] > 1 && result[result[0]] == 0) @@ -1161,7 +1161,8 @@ Bignum bignum_from_bytes_le(const unsigned char *data, int nbytes) result[i] = 0; for (i = 0; i < nbytes; ++i) { unsigned char byte = *data++; - result[1 + i / BIGNUM_INT_BYTES] |= byte << (8*i % BIGNUM_INT_BITS); + result[1 + i / BIGNUM_INT_BYTES] |= + (BignumInt)byte << (8*i % BIGNUM_INT_BITS); } while (result[0] > 1 && result[result[0]] == 0) @@ -1314,7 +1315,7 @@ void bignum_set_bit(Bignum bn, int bitnum, int value) abort(); /* beyond the end */ else { int v = bitnum / BIGNUM_INT_BITS + 1; - int mask = 1 << (bitnum % BIGNUM_INT_BITS); + BignumInt mask = (BignumInt)1 << (bitnum % BIGNUM_INT_BITS); if (value) bn[v] |= mask; else diff --git a/sshccp.c b/sshccp.c index df973e63..71fde427 100644 --- a/sshccp.c +++ b/sshccp.c @@ -198,7 +198,8 @@ static void bigval_import_le(bigval *r, const void *vdata, int len) int i; bigval_clear(r); for (i = 0; i < len; i++) - r->w[i / BIGNUM_INT_BYTES] |= data[i] << (8 * (i % BIGNUM_INT_BYTES)); + r->w[i / BIGNUM_INT_BYTES] |= + (BignumInt)data[i] << (8 * (i % BIGNUM_INT_BYTES)); } static void bigval_export_le(const bigval *r, void *vdata, int len) @@ -951,7 +952,8 @@ static void poly1305_feed_chunk(struct poly1305 *ctx, { bigval c; bigval_import_le(&c, chunk, len); - c.w[len / BIGNUM_INT_BYTES] |= 1 << (8 * (len % BIGNUM_INT_BYTES)); + c.w[len / BIGNUM_INT_BYTES] |= + (BignumInt)1 << (8 * (len % BIGNUM_INT_BYTES)); bigval_add(&c, &c, &ctx->h); bigval_mul_mod_p(&ctx->h, &c, &ctx->r); }