Do not accept EC signatures when the public point is invalid

Force g*x+p*y to the point at infinity if g or p is not a valid point. This does not seem to be a security issue since if you can control the public point you can just use a regular key and sign whatever you like. Still, it's odd to accept anything here, and easy to prevent it. Found by CryptoFuzz reported offline by @guidovranken
randombit · Nov 30, 2020 · 92cd9ad · 92cd9ad
1 parent 3415450
commit 92cd9ad
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 2 deletions.
diff --git a/src/lib/pubkey/ec_group/point_mul.cpp b/src/lib/pubkey/ec_group/point_mul.cpp
@@ -321,6 +321,12 @@ PointGFp PointGFp_Var_Point_Precompute::mul(const BigInt& k,
 PointGFp_Multi_Point_Precompute::PointGFp_Multi_Point_Precompute(const PointGFp& x,
                                                                  const PointGFp& y)
    {
+   if(x.on_the_curve() == false || y.on_the_curve() == false)
+      {
+      m_M.push_back(x.zero());
+      return;
+      }
+
    std::vector<BigInt> ws(PointGFp::WORKSPACE_SIZE);
 
    PointGFp x2 = x;
@@ -372,6 +378,9 @@ PointGFp_Multi_Point_Precompute::PointGFp_Multi_Point_Precompute(const PointGFp&
 PointGFp PointGFp_Multi_Point_Precompute::multi_exp(const BigInt& z1,
                                                     const BigInt& z2) const
    {
+   if(m_M.size() == 1)
+      return m_M[0];
+
    std::vector<BigInt> ws(PointGFp::WORKSPACE_SIZE);
 
    const size_t z_bits = round_up(std::max(z1.bits(), z2.bits()), 2);

diff --git a/src/tests/data/pubkey/ecdsa_verify.vec b/src/tests/data/pubkey/ecdsa_verify.vec
@@ -68,3 +68,11 @@ Px = 115014969141122710336858256331515905188079709631033705848897690363377891
 Py = 81333916963110019576228330948951168219884247801435258672405011123948094
 Msg = 0679246D6C4216DE0DAA08E5523FB2674DB2B6599C3B72FF946B488A15290B62
 Signature = 30cf3ae9da8c18ef37664e358e43b07f93ded599653e64acd171e197a1c72f9ad521e5e2e091e9fe4c27f1110265ec5cbb701a6faf3569304774de5f
+
+# From CryptoFuzz
+Group = secp256k1
+Valid = 0
+Px = 70000000000000000
+Py = 0
+Msg = d1a0b2537473db2edf298bab7cb968882ab9d558c2a6298b4556ee9d36298cae
+Signature = 79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798D1A0B2537473DB2EDF298BAB7CB968882AB9D558C2A6298B4556EE9D36298CAE
diff --git a/src/tests/test_ecdsa.cpp b/src/tests/test_ecdsa.cpp
@@ -27,7 +27,8 @@ class ECDSA_Verification_Tests final : public PK_Signature_Verification_Test
       ECDSA_Verification_Tests() : PK_Signature_Verification_Test(
             "ECDSA",
             "pubkey/ecdsa_verify.vec",
-            "Group,Px,Py,Hash,Msg,Signature") {}
+            "Group,Px,Py,Hash,Msg,Signature",
+            "Valid") {}
 
       bool clear_between_callbacks() const override
          {

diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
@@ -237,7 +237,9 @@ PK_Signature_Verification_Test::run_one_test(const std::string& pad_hdr, const V
                   }
                }
             else
-               result.test_eq("incorrect signature invalid", verified, false);
+               {
+               result.confirm("incorrect signature is rejected", verified == false);
+               }
             }
          catch(std::exception& e)
             {