diff --git a/src/field.h b/src/field.h index 64ceead4d2..c7a88c4e13 100644 --- a/src/field.h +++ b/src/field.h @@ -75,7 +75,9 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b); /** Compare two field elements. Requires both inputs to be normalized */ static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b); -/** Set a field element equal to 32-byte big endian value. If successful, the resulting field element is normalized. */ +/** Set a field element equal to 32-byte big endian value. + * Returns 1 if no overflow occurred, and then the output is normalized. + * Returns 0 if overflow occurred, and then the output is only weakly normalized. */ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a); /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */ diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index 3b7f4d2480..6b58380f98 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -367,12 +367,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { ret = !((r->n[9] == 0x3FFFFFUL) & ((r->n[8] & r->n[7] & r->n[6] & r->n[5] & r->n[4] & r->n[3] & r->n[2]) == 0x3FFFFFFUL) & ((r->n[1] + 0x40UL + ((r->n[0] + 0x3D1UL) >> 26)) > 0x3FFFFFFUL)); #ifdef VERIFY r->magnitude = 1; - if (ret) { - r->normalized = 1; - secp256k1_fe_verify(r); - } else { - r->normalized = 0; - } + r->normalized = ret; + secp256k1_fe_verify(r); #endif return ret; } diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 6b97157d0f..0e403f32a6 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -342,12 +342,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { ret = !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL)); #ifdef VERIFY r->magnitude = 1; - if (ret) { - r->normalized = 1; - secp256k1_fe_verify(r); - } else { - r->normalized = 0; - } + r->normalized = ret; + secp256k1_fe_verify(r); #endif return ret; } diff --git a/src/tests.c b/src/tests.c index 9b3c92dd6e..0f9c35faf2 100644 --- a/src/tests.c +++ b/src/tests.c @@ -44,6 +44,25 @@ static int all_bytes_equal(const void* s, unsigned char value, size_t n) { return 1; } +/* Debug helper for printing arrays of unsigned char. */ +#define PRINT_BUF(buf, len) do { \ + printf("%s[%lu] = ", #buf, (unsigned long)len); \ + print_buf_plain(buf, len); \ +} while(0); +static void print_buf_plain(const unsigned char *buf, size_t len) { + size_t i; + printf("{"); + for (i = 0; i < len; i++) { + if (i % 8 == 0) { + printf("\n "); + } else { + printf(" "); + } + printf("0x%02X,", buf[i]); + } + printf("\n}\n"); +} + /* TODO Use CHECK_ILLEGAL(_VOID) everywhere and get rid of the uncounting callback */ /* CHECK that expr_or_stmt calls the illegal callback of ctx exactly once * @@ -3027,6 +3046,69 @@ static void run_field_convert(void) { CHECK(secp256k1_memcmp_var(&fes2, &fes, sizeof(fes)) == 0); } +static void run_field_be32_overflow(void) { + { + static const unsigned char zero_overflow[32] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x2F, + }; + static const unsigned char zero[32] = { 0x00 }; + unsigned char out[32]; + secp256k1_fe fe; + CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0); + CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1); + secp256k1_fe_normalize(&fe); + CHECK(secp256k1_fe_is_zero(&fe) == 1); + secp256k1_fe_get_b32(out, &fe); + CHECK(secp256k1_memcmp_var(out, zero, 32) == 0); + } + { + static const unsigned char one_overflow[32] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x30, + }; + static const unsigned char one[32] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + }; + unsigned char out[32]; + secp256k1_fe fe; + CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0); + secp256k1_fe_normalize(&fe); + CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0); + secp256k1_fe_get_b32(out, &fe); + CHECK(secp256k1_memcmp_var(out, one, 32) == 0); + } + { + static const unsigned char ff_overflow[32] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + }; + static const unsigned char ff[32] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x03, 0xD0, + }; + unsigned char out[32]; + secp256k1_fe fe; + const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0); + CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0); + secp256k1_fe_normalize(&fe); + CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0); + secp256k1_fe_get_b32(out, &fe); + CHECK(secp256k1_memcmp_var(out, ff, 32) == 0); + } +} + /* Returns true if two field elements have the same representation. */ static int fe_identical(const secp256k1_fe *a, const secp256k1_fe *b) { int ret = 1; @@ -7693,6 +7775,7 @@ int main(int argc, char **argv) { run_field_half(); run_field_misc(); run_field_convert(); + run_field_be32_overflow(); run_fe_mul(); run_sqr(); run_sqrt();