fix invalid tx signature v not checked

bxq2011hust · Dec 6, 2023 · b7c9c8f · b7c9c8f
1 parent 340d536
commit b7c9c8f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 3 deletions.
diff --git a/.github/workflows/workflow-self-hosted-arm.yml b/.github/workflows/workflow-self-hosted-arm.yml
@@ -61,7 +61,7 @@ jobs:
 
       - name: update vcpkg
         run: |
-          cd ${{ env.VCPKG_ROOT }} && git fetch --all && git checkout master && git pull
+          cd ${{ env.VCPKG_ROOT }} && git fetch --all
           cd -
 
       - name: Build for linux

diff --git a/bcos-crypto/bcos-crypto/signature/secp256k1/Secp256k1Crypto.cpp b/bcos-crypto/bcos-crypto/signature/secp256k1/Secp256k1Crypto.cpp
@@ -86,6 +86,14 @@ bool bcos::crypto::secp256k1Verify(
     return false;
 #endif
 
+    if ((uint8_t)_signatureData[SECP256K1_SIGNATURE_V] > 3)
+    {
+        BOOST_THROW_EXCEPTION(
+            InvalidSignature() << errinfo_comment(
+                "secp256k1 verify illegal argument: recid >= 0 && recid <= 3, recid: " +
+                std::to_string((int)_signatureData[SECP256K1_SIGNATURE_V])));
+    }
+
     secp256k1_ecdsa_recoverable_signature sig;
     secp256k1_ecdsa_recoverable_signature_parse_compact(g_SECP256K1_CTX.get(), &sig,
         _signatureData.data(), (int)_signatureData[SECP256K1_SIGNATURE_V]);
@@ -155,11 +163,11 @@ PublicPtr bcos::crypto::secp256k1Recover(const HashType& _hash, bytesConstRef _s
     }
     return pubKey;
 #endif
-    if ((int)_signatureData[SECP256K1_SIGNATURE_V] > 3)
+    if ((uint8_t)_signatureData[SECP256K1_SIGNATURE_V] > 3)
     {
         BOOST_THROW_EXCEPTION(
             InvalidSignature() << errinfo_comment(
-                "secp256k1Sign illegal argument: recid >= 0 && recid <= 3, recid: " +
+                "secp256k1 recover illegal argument: recid >= 0 && recid <= 3, recid: " +
                 std::to_string((int)_signatureData[SECP256K1_SIGNATURE_V])));
     }
     auto pubKey = std::make_shared<KeyImpl>(SECP256K1_PUBLIC_LEN);

diff --git a/bcos-crypto/test/unittests/SignatureTest.cpp b/bcos-crypto/test/unittests/SignatureTest.cpp
@@ -191,6 +191,14 @@ BOOST_AUTO_TEST_CASE(testSecp256k1SignAndVerify)
     BOOST_CHECK(*signData == *encodedData);
     auto publicKey = secp256k1Crypto->recover(hashData, ref(*encodedData));
     BOOST_CHECK(publicKey->data() == keyPair->publicKey()->data());
+    for (uint8_t i = 4; i < 255; i++)
+    {
+        (*encodedData)[SECP256K1_SIGNATURE_V] = i;
+        BOOST_CHECK_THROW(secp256k1Crypto->recover(hashData, ref(*encodedData)), InvalidSignature);
+        BOOST_CHECK_THROW(
+            secp256k1Crypto->verify(keyPair->publicKey(), hashData, ref(*encodedData)),
+            InvalidSignature);
+    }
 }
 
 BOOST_AUTO_TEST_CASE(testSM2KeyPair)