Merge pull request #114

99f0728 Fix secp256k1_num_set_bin handling of 0 (Pieter Wuille)
d907ebc Add bounds checking to field element setters (Pieter Wuille)

src/ecdsa_impl.h

@@ -108,7 +108,7 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_ecdsa_sig_t *sig, secp256
     secp256k1_num_get_bin(brx, 32, &rx);
     secp256k1_num_free(&rx);
     secp256k1_fe_t fx;
-    secp256k1_fe_set_b32(&fx, brx);
+    VERIFY_CHECK(secp256k1_fe_set_b32(&fx, brx)); /* Either rx < n (and n < p), or rx + n < p (checked above). */
     secp256k1_ge_t x;
     if (!secp256k1_ge_set_xo(&x, &fx, recid & 1))
         return 0;

src/eckey_impl.h

@@ -17,12 +17,12 @@
 static int secp256k1_eckey_pubkey_parse(secp256k1_ge_t *elem, const unsigned char *pub, int size) {
     if (size == 33 && (pub[0] == 0x02 || pub[0] == 0x03)) {
         secp256k1_fe_t x;
-        secp256k1_fe_set_b32(&x, pub+1);
+        return secp256k1_fe_set_b32(&x, pub+1) && secp256k1_ge_set_xo(elem, &x, pub[0] == 0x03);
-        return secp256k1_ge_set_xo(elem, &x, pub[0] == 0x03);
     } else if (size == 65 && (pub[0] == 0x04 || pub[0] == 0x06 || pub[0] == 0x07)) {
         secp256k1_fe_t x, y;
-        secp256k1_fe_set_b32(&x, pub+1);
+        if (!secp256k1_fe_set_b32(&x, pub+1) || !secp256k1_fe_set_b32(&y, pub+33)) {
-        secp256k1_fe_set_b32(&y, pub+33);
+            return 0;
+        }
         secp256k1_ge_set_xy(elem, &x, &y);
         if ((pub[0] == 0x06 || pub[0] == 0x07) && secp256k1_fe_is_odd(&y) != (pub[0] == 0x07))
             return 0;

src/ecmult_gen_impl.h

@@ -45,7 +45,7 @@ static void secp256k1_ecmult_gen_start(void) {
     {
         static const unsigned char nums_b32[32] = "The scalar for this x is unknown";
         secp256k1_fe_t nums_x;
-        secp256k1_fe_set_b32(&nums_x, nums_b32);
+        VERIFY_CHECK(secp256k1_fe_set_b32(&nums_x, nums_b32));
         secp256k1_ge_t nums_ge;
         VERIFY_CHECK(secp256k1_ge_set_xo(&nums_ge, &nums_x, 0));
         secp256k1_gej_set_ge(&nums_gej, &nums_ge);

src/field.h

@@ -59,8 +59,8 @@ static int secp256k1_fe_is_odd(const secp256k1_fe_t *a);
 /** Compare two field elements. Requires both inputs to be normalized */
 static int secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe_t *b);
 
-/** Set a field element equal to 32-byte big endian value. Resulting field element is normalized. */
+/** Set a field element equal to 32-byte big endian value. If succesful, the resulting field element is normalized. */
-static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a);
+static int secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a);
 
 /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */
 static void secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe_t *a);
@@ -109,6 +109,6 @@ static void secp256k1_fe_inv_all_var(size_t len, secp256k1_fe_t r[len], const se
 static void secp256k1_fe_get_hex(char *r, int *rlen, const secp256k1_fe_t *a);
 
 /** Convert a hexadecimal string to a field element. */
-static void secp256k1_fe_set_hex(secp256k1_fe_t *r, const char *a, int alen);
+static int secp256k1_fe_set_hex(secp256k1_fe_t *r, const char *a, int alen);
 
 #endif

src/field_10x26_impl.h

@@ -152,7 +152,7 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe_t *a, const se
           | (t[5]^u[5]) | (t[6]^u[6]) | (t[7]^u[7]) | (t[8]^u[8]) | (t[9]^u[9])) == 0;
 }
 
-static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
+static int secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
     r->n[0] = r->n[1] = r->n[2] = r->n[3] = r->n[4] = 0;
     r->n[5] = r->n[6] = r->n[7] = r->n[8] = r->n[9] = 0;
     for (int i=0; i<32; i++) {
@@ -162,11 +162,15 @@ static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
             r->n[limb] |= (uint32_t)((a[31-i] >> (2*j)) & 0x3) << shift;
         }
     }
+    if (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) {
+        return 0;
+    }
 #ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 1;
     secp256k1_fe_verify(r);
 #endif
+    return 1;
 }
 
 /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */

src/field_5x52_impl.h

@@ -150,7 +150,7 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe_t *a, const se
     return ((t[0]^u[0]) | (t[1]^u[1]) | (t[2]^u[2]) | (t[3]^u[3]) | (t[4]^u[4])) == 0;
 }
 
-static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
+static int secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
     r->n[0] = r->n[1] = r->n[2] = r->n[3] = r->n[4] = 0;
     for (int i=0; i<32; i++) {
         for (int j=0; j<2; j++) {
@@ -159,11 +159,15 @@ static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
             r->n[limb] |= (uint64_t)((a[31-i] >> (4*j)) & 0xF) << shift;
         }
     }
+    if (r->n[4] == 0x0FFFFFFFFFFFFULL && (r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL && r->n[0] >= 0xFFFFEFFFFFC2FULL) {
+        return 0;
+    }
 #ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 1;
     secp256k1_fe_verify(r);
 #endif
+    return 1;
 }
 
 /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */

src/field_gmp_impl.h

@@ -75,7 +75,7 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe_t *a, const se
     return ret;
 }
 
-static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
+static int secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
     for (int i=0; i<FIELD_LIMBS+1; i++)
         r->n[i] = 0;
     for (int i=0; i<256; i++) {
@@ -83,6 +83,7 @@ static void secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
         int shift = i%GMP_NUMB_BITS;
         r->n[limb] |= (mp_limb_t)((a[31-i/8] >> (i%8)) & 0x1) << shift;
     }
+    return (mpn_cmp(r->n, secp256k1_field_p, FIELD_LIMBS) < 0);
 }
 
 /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */

src/field_impl.h

@@ -41,7 +41,7 @@ static void secp256k1_fe_get_hex(char *r, int *rlen, const secp256k1_fe_t *a) {
     r[64] = 0x00;
 }
 
-static void secp256k1_fe_set_hex(secp256k1_fe_t *r, const char *a, int alen) {
+static int secp256k1_fe_set_hex(secp256k1_fe_t *r, const char *a, int alen) {
     unsigned char tmp[32] = {};
     static const int cvt[256] = {0, 0, 0, 0, 0, 0, 0,0,0,0,0,0,0,0,0,0,
                                  0, 0, 0, 0, 0, 0, 0,0,0,0,0,0,0,0,0,0,
@@ -63,7 +63,7 @@ static void secp256k1_fe_set_hex(secp256k1_fe_t *r, const char *a, int alen) {
         if (alen > i*2)
             tmp[32 - alen/2 + i] = (cvt[(unsigned char)a[2*i]] << 4) + cvt[(unsigned char)a[2*i+1]];
     }
-    secp256k1_fe_set_b32(r, tmp);
+    return secp256k1_fe_set_b32(r, tmp);
 }
 
 static int secp256k1_fe_sqrt(secp256k1_fe_t *r, const secp256k1_fe_t *a) {
@@ -212,7 +212,7 @@ static void secp256k1_fe_inv_var(secp256k1_fe_t *r, const secp256k1_fe_t *a) {
     secp256k1_num_set_bin(&n, b, 32);
     secp256k1_num_mod_inverse(&n, &n, &secp256k1_fe_consts->p);
     secp256k1_num_get_bin(b, 32, &n);
-    secp256k1_fe_set_b32(r, b);
+    VERIFY_CHECK(secp256k1_fe_set_b32(r, b));
 #else
 #error "Please select field inverse implementation"
 #endif

src/group_impl.h

@@ -498,11 +498,11 @@ static void secp256k1_ge_start(void) {
         secp256k1_num_set_bin(&ret->a1b2,   secp256k1_ge_consts_a1b2,   sizeof(secp256k1_ge_consts_a1b2));
         secp256k1_num_set_bin(&ret->a2,     secp256k1_ge_consts_a2,     sizeof(secp256k1_ge_consts_a2));
         secp256k1_num_set_bin(&ret->b1,     secp256k1_ge_consts_b1,     sizeof(secp256k1_ge_consts_b1));
-        secp256k1_fe_set_b32(&ret->beta, secp256k1_ge_consts_beta);
+        VERIFY_CHECK(secp256k1_fe_set_b32(&ret->beta, secp256k1_ge_consts_beta));
 #endif
         secp256k1_fe_t g_x, g_y;
-        secp256k1_fe_set_b32(&g_x, secp256k1_ge_consts_g_x);
+        VERIFY_CHECK(secp256k1_fe_set_b32(&g_x, secp256k1_ge_consts_g_x));
-        secp256k1_fe_set_b32(&g_y, secp256k1_ge_consts_g_y);
+        VERIFY_CHECK(secp256k1_fe_set_b32(&g_y, secp256k1_ge_consts_g_y));
         secp256k1_ge_set_xy(&ret->g, &g_x, &g_y);
         secp256k1_ge_consts = ret;
     }

src/num_gmp_impl.h

@@ -71,6 +71,10 @@ static void secp256k1_num_set_bin(secp256k1_num_t *r, const unsigned char *a, un
     VERIFY_CHECK(alen > 0);
     VERIFY_CHECK(alen <= 64);
     int len = mpn_set_str(r->data, a, alen, 256);
+    if (len == 0) {
+        r->data[0] = 0;
+        len = 1;
+    }
     VERIFY_CHECK(len <= NUM_LIMBS*2);
     r->limbs = len;
     r->neg = 0;

src/tests.c

@@ -38,7 +38,7 @@ void random_field_element_test(secp256k1_fe_t *fe) {
         secp256k1_num_set_bin(&num, b32, 32);
         if (secp256k1_num_cmp(&num, &secp256k1_fe_consts->p) >= 0)
             continue;
-        secp256k1_fe_set_b32(fe, b32);
+        VERIFY_CHECK(secp256k1_fe_set_b32(fe, b32));
         break;
     } while(1);
 }
@@ -440,8 +440,12 @@ void run_scalar_tests(void) {
 
 void random_fe(secp256k1_fe_t *x) {
     unsigned char bin[32];
+    do {
         secp256k1_rand256(bin);
-    secp256k1_fe_set_b32(x, bin);
+        if (secp256k1_fe_set_b32(x, bin)) {
+            return;
+        }
+    } while(1);
 }
 
 void random_fe_non_zero(secp256k1_fe_t *nz) {
@@ -697,8 +701,8 @@ void run_ge(void) {
 
 void run_ecmult_chain(void) {
     /* random starting point A (on the curve) */
-    secp256k1_fe_t ax; secp256k1_fe_set_hex(&ax, "8b30bbe9ae2a990696b22f670709dff3727fd8bc04d3362c6c7bf458e2846004", 64);
+    secp256k1_fe_t ax; VERIFY_CHECK(secp256k1_fe_set_hex(&ax, "8b30bbe9ae2a990696b22f670709dff3727fd8bc04d3362c6c7bf458e2846004", 64));
-    secp256k1_fe_t ay; secp256k1_fe_set_hex(&ay, "a357ae915c4a65281309edf20504740f0eb3343990216b4f81063cb65f2f7e0f", 64);
+    secp256k1_fe_t ay; VERIFY_CHECK(secp256k1_fe_set_hex(&ay, "a357ae915c4a65281309edf20504740f0eb3343990216b4f81063cb65f2f7e0f", 64));
     secp256k1_gej_t a; secp256k1_gej_set_xy(&a, &ax, &ay);
     /* two random initial factors xn and gn */
     secp256k1_num_t xn;
@@ -759,7 +763,7 @@ void test_point_times_order(const secp256k1_gej_t *point) {
 }
 
 void run_point_times_order(void) {
-    secp256k1_fe_t x; secp256k1_fe_set_hex(&x, "02", 2);
+    secp256k1_fe_t x; VERIFY_CHECK(secp256k1_fe_set_hex(&x, "02", 2));
     for (int i=0; i<500; i++) {
         secp256k1_ge_t p;
         if (secp256k1_ge_set_xo(&p, &x, 1)) {