Merge pull request #110

3bf029d Add test that recovering infinity fails (Pieter Wuille)
4861f83 Test whether recovered public keys are not infinity (Pieter Wuille)
bbe67d8 Make secp256k1_eckey_pubkey_serialize fail for infinity (Pieter Wuille)

src/ecdsa_impl.h

@@ -128,7 +128,7 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_ecdsa_sig_t *sig, secp256
     secp256k1_num_free(&rn);
     secp256k1_num_free(&u1);
     secp256k1_num_free(&u2);
-    return 1;
+    return !secp256k1_gej_is_infinity(&qj);
 }
 
 static int secp256k1_ecdsa_sig_verify(const secp256k1_ecdsa_sig_t *sig, const secp256k1_ge_t *pubkey, const secp256k1_num_t *message) {

src/eckey.h

@@ -12,7 +12,7 @@
 #include "num.h"
 
 static int secp256k1_eckey_pubkey_parse(secp256k1_ge_t *elem, const unsigned char *pub, int size);
-static void secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char *pub, int *size, int compressed);
+static int secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char *pub, int *size, int compressed);
 
 static int secp256k1_eckey_privkey_parse(secp256k1_scalar_t *key, const unsigned char *privkey, int privkeylen);
 static int secp256k1_eckey_privkey_serialize(unsigned char *privkey, int *privkeylen, const secp256k1_scalar_t *key, int compressed);

src/eckey_impl.h

@@ -32,7 +32,10 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge_t *elem, const unsigned cha
     }
 }
 
-static void secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char *pub, int *size, int compressed) {
+static int secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char *pub, int *size, int compressed) {
+    if (secp256k1_ge_is_infinity(elem)) {
+        return 0;
+    }
     secp256k1_fe_normalize(&elem->x);
     secp256k1_fe_normalize(&elem->y);
     secp256k1_fe_get_b32(&pub[1], &elem->x);
@@ -44,6 +47,7 @@ static void secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char
         pub[0] = 0x04;
         secp256k1_fe_get_b32(&pub[33], &elem->y);
     }
+    return 1;
 }
 
 static int secp256k1_eckey_privkey_parse(secp256k1_scalar_t *key, const unsigned char *privkey, int privkeylen) {
@@ -107,7 +111,10 @@ static int secp256k1_eckey_privkey_serialize(unsigned char *privkey, int *privke
         secp256k1_scalar_get_b32(ptr, key); ptr += 32;
         memcpy(ptr, middle, sizeof(middle)); ptr += sizeof(middle);
         int pubkeylen = 0;
-        secp256k1_eckey_pubkey_serialize(&r, ptr, &pubkeylen, 1); ptr += pubkeylen;
+        if (!secp256k1_eckey_pubkey_serialize(&r, ptr, &pubkeylen, 1)) {
+            return 0;
+        }
+        ptr += pubkeylen;
         *privkeylen = ptr - privkey;
     } else {
         static const unsigned char begin[] = {
@@ -131,7 +138,10 @@ static int secp256k1_eckey_privkey_serialize(unsigned char *privkey, int *privke
         secp256k1_scalar_get_b32(ptr, key); ptr += 32;
         memcpy(ptr, middle, sizeof(middle)); ptr += sizeof(middle);
         int pubkeylen = 0;
-        secp256k1_eckey_pubkey_serialize(&r, ptr, &pubkeylen, 0); ptr += pubkeylen;
+        if (!secp256k1_eckey_pubkey_serialize(&r, ptr, &pubkeylen, 0)) {
+            return 0;
+        }
+        ptr += pubkeylen;
         *privkeylen = ptr - privkey;
     }
     return 1;

src/secp256k1.c

@@ -150,8 +150,7 @@ int secp256k1_ecdsa_recover_compact(const unsigned char *msg, int msglen, const
 
     secp256k1_ge_t q;
     if (secp256k1_ecdsa_sig_recover(&sig, &q, &m, recid)) {
-        secp256k1_eckey_pubkey_serialize(&q, pubkey, pubkeylen, compressed);
-        ret = 1;
+        ret = secp256k1_eckey_pubkey_serialize(&q, pubkey, pubkeylen, compressed);
     }
     return ret;
 }
@@ -187,8 +186,7 @@ int secp256k1_ec_pubkey_create(unsigned char *pubkey, int *pubkeylen, const unsi
     secp256k1_scalar_clear(&sec);
     secp256k1_ge_t p;
     secp256k1_ge_set_gej(&p, &pj);
-    secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, compressed);
-    return 1;
+    return secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, compressed);
 }
 
 int secp256k1_ec_pubkey_decompress(unsigned char *pubkey, int *pubkeylen) {
@@ -198,8 +196,7 @@ int secp256k1_ec_pubkey_decompress(unsigned char *pubkey, int *pubkeylen) {
     secp256k1_ge_t p;
     if (!secp256k1_eckey_pubkey_parse(&p, pubkey, *pubkeylen))
         return 0;
-    secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, 0);
-    return 1;
+    return secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, 0);
 }
 
 int secp256k1_ec_privkey_tweak_add(unsigned char *seckey, const unsigned char *tweak) {
@@ -236,7 +233,7 @@ int secp256k1_ec_pubkey_tweak_add(unsigned char *pubkey, int pubkeylen, const un
     }
     if (ret) {
         int oldlen = pubkeylen;
-        secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
+        ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
         VERIFY_CHECK(pubkeylen == oldlen);
     }
 
@@ -276,7 +273,7 @@ int secp256k1_ec_pubkey_tweak_mul(unsigned char *pubkey, int pubkeylen, const un
     }
     if (ret) {
         int oldlen = pubkeylen;
-        secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
+        ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
         VERIFY_CHECK(pubkeylen == oldlen);
     }
 

src/tests.c

@@ -935,6 +935,36 @@ void run_ecdsa_end_to_end(void) {
     }
 }
 
+void test_ecdsa_infinity(void) {
+    const unsigned char msg32[32] = {
+        'T', 'h', 'i', 's', ' ', 'i', 's', ' ',
+        'a', ' ', 'v', 'e', 'r', 'y', ' ', 's',
+        'e', 'c', 'r', 'e', 't', ' ', 'm', 'e',
+        's', 's', 'a', 'g', 'e', '.', '.', '.'
+    };
+    const unsigned char sig64[64] = {
+        // Generated by signing the above message with nonce 'This is the nonce we will use...'
+        // and secret key 0 (which is not valid), resulting in recid 0.
+        0x67, 0xCB, 0x28, 0x5F, 0x9C, 0xD1, 0x94, 0xE8,
+        0x40, 0xD6, 0x29, 0x39, 0x7A, 0xF5, 0x56, 0x96,
+        0x62, 0xFD, 0xE4, 0x46, 0x49, 0x99, 0x59, 0x63,
+        0x17, 0x9A, 0x7D, 0xD1, 0x7B, 0xD2, 0x35, 0x32,
+        0x4B, 0x1B, 0x7D, 0xF3, 0x4C, 0xE1, 0xF6, 0x8E,
+        0x69, 0x4F, 0xF6, 0xF1, 0x1A, 0xC7, 0x51, 0xDD,
+        0x7D, 0xD7, 0x3E, 0x38, 0x7E, 0xE4, 0xFC, 0x86,
+        0x6E, 0x1B, 0xE8, 0xEC, 0xC7, 0xDD, 0x95, 0x57
+    };
+    unsigned char pubkey[65];
+    int pubkeylen = 65;
+    CHECK(!secp256k1_ecdsa_recover_compact(msg32, 32, sig64, pubkey, &pubkeylen, 0, 0));
+    CHECK(secp256k1_ecdsa_recover_compact(msg32, 32, sig64, pubkey, &pubkeylen, 0, 1));
+    CHECK(!secp256k1_ecdsa_recover_compact(msg32, 32, sig64, pubkey, &pubkeylen, 0, 2));
+    CHECK(!secp256k1_ecdsa_recover_compact(msg32, 32, sig64, pubkey, &pubkeylen, 0, 3));
+}
+
+void run_ecdsa_infinity(void) {
+    test_ecdsa_infinity();
+}
 
 #ifdef ENABLE_OPENSSL_TESTS
 EC_KEY *get_openssl_key(const secp256k1_scalar_t *key) {
@@ -1037,6 +1067,7 @@ int main(int argc, char **argv) {
     /* ecdsa tests */
     run_ecdsa_sign_verify();
     run_ecdsa_end_to_end();
+    run_ecdsa_infinity();
 #ifdef ENABLE_OPENSSL_TESTS
     run_ecdsa_openssl();
 #endif