ecdsa_adaptor: fix test case with invalid signature

Previously the ECDSA signature had an overflowing s value, which after the sync with upstream results in a failing VERIFY_CHECK in the inversion function. However, normally parsed signatures shouldn't contain overflowing s values.
BlockstreamResearch · Jul 13, 2021 · b053e85 · b053e85
1 parent d27e459
commit b053e85
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 12 deletions.
diff --git a/src/modules/ecdsa_adaptor/main_impl.h b/src/modules/ecdsa_adaptor/main_impl.h
@@ -341,6 +341,17 @@ int secp256k1_ecdsa_adaptor_recover(const secp256k1_context* ctx, unsigned char
      * branch point. */
     secp256k1_declassify(ctx, &enckey_expected_ge, sizeof(enckey_expected_ge));
     if (!secp256k1_eckey_pubkey_serialize(&enckey_expected_ge, enckey_expected33, &size, SECP256K1_EC_COMPRESSED)) {
+        /* Unreachable from tests (and other VERIFY builds) and therefore this
+         * branch should be ignored in test coverage analysis.
+         *
+         * Proof:
+         *     eckey_pubkey_serialize fails <=> deckey = 0
+         *     deckey = 0 <=> s^-1 = 0 or sp = 0
+         *     case 1: s^-1 = 0 impossible by the definition of multiplicative
+         *             inverse and because the scalar_inverse implementation
+         *             VERIFY_CHECKs that the inputs are valid scalars.
+         *     case 2: sp = 0 impossible because ecdsa_adaptor_sig_deserialize would have already failed
+         */
         return 0;
     }
     if (!secp256k1_ec_pubkey_serialize(ctx, enckey33, &size, enckey, SECP256K1_EC_COMPRESSED)) {

diff --git a/src/modules/ecdsa_adaptor/tests_impl.h b/src/modules/ecdsa_adaptor/tests_impl.h
@@ -1102,15 +1102,8 @@ void adaptor_tests(void) {
     }
     {
         /* Test key recover */
-        secp256k1_ecdsa_signature sig_tmp;
         unsigned char decryption_key_tmp[32];
         unsigned char adaptor_sig_tmp[162];
-        const unsigned char order_le[32] = {
-            0x41, 0x41, 0x36, 0xd0, 0x8c, 0x5e, 0xd2, 0xbf,
-            0x3b, 0xa0, 0x48, 0xaf, 0xe6, 0xdc, 0xae, 0xba,
-            0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
-        };
 
         CHECK(secp256k1_ecdsa_adaptor_recover(ctx, decryption_key_tmp, &sig, adaptor_sig, &enckey) == 1);
         CHECK(secp256k1_memcmp_var(deckey, decryption_key_tmp, sizeof(deckey)) == 0);
@@ -1119,11 +1112,6 @@ void adaptor_tests(void) {
         memcpy(adaptor_sig_tmp, adaptor_sig, sizeof(adaptor_sig_tmp));
         memset(&adaptor_sig_tmp[66], 0xFF, 32);
         CHECK(secp256k1_ecdsa_adaptor_recover(ctx, decryption_key_tmp, &sig, adaptor_sig_tmp, &enckey) == 0);
-
-        /* Test failed enckey_expected serialization */
-        memcpy(sig_tmp.data, sig.data, 32);
-        memcpy(&sig_tmp.data[32], order_le, 32);
-        CHECK(secp256k1_ecdsa_adaptor_recover(ctx, decryption_key_tmp, &sig_tmp, adaptor_sig, &enckey) == 0);
     }
 }