From 593ff7e92947abe5827d2fb8f918e9ed5cc7d96a Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Sat, 11 Mar 2023 19:44:35 +0200 Subject: [PATCH] fix: governance correct sig check (#5242) ## Issue being fixed or feature implemented ## What was done? When verifying signature of `CGovernanceVote`/`CGovernanceObject` we need to use the active scheme. ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation **For repository code-owners and collaborators only** - [x] I have assigned this pull request to a milestone --- src/bls/bls.cpp | 9 +++++++-- src/bls/bls.h | 2 +- src/governance/object.cpp | 2 +- src/governance/vote.cpp | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index fc1965d21d..0d0b04451a 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -288,19 +288,24 @@ void CBLSSignature::SubInsecure(const CBLSSignature& o) cachedHash.SetNull(); } -bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const +bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash, const bool specificLegacyScheme) const { if (!IsValid() || !pubKey.IsValid()) { return false; } try { - return Scheme(bls::bls_legacy_scheme.load())->Verify(pubKey.impl, bls::Bytes(hash.begin(), hash.size()), impl); + return Scheme(specificLegacyScheme)->Verify(pubKey.impl, bls::Bytes(hash.begin(), hash.size()), impl); } catch (...) { return false; } } +bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const +{ + return VerifyInsecure(pubKey, hash, bls::bls_legacy_scheme.load()); +} + bool CBLSSignature::VerifyInsecureAggregated(const std::vector& pubKeys, const std::vector& hashes) const { if (!IsValid()) { diff --git a/src/bls/bls.h b/src/bls/bls.h index b74efd33fd..b160df722a 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -368,7 +368,7 @@ public: static CBLSSignature AggregateSecure(const std::vector& sigs, const std::vector& pks, const uint256& hash); void SubInsecure(const CBLSSignature& o); - + [[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash, const bool specificLegacyScheme) const; [[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const; [[nodiscard]] bool VerifyInsecureAggregated(const std::vector& pubKeys, const std::vector& hashes) const; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 5bb57ff08b..61b1043355 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -318,7 +318,7 @@ bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->nTime; sig.SetByteVector(vchSig, is_bls_legacy_scheme); - if (!sig.VerifyInsecure(pubKey, GetSignatureHash())) { + if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) { LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n"); return false; } diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index f5dd9beed9..8482ed29bc 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -240,7 +240,7 @@ bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->nTime; sig.SetByteVector(vchSig, is_bls_legacy_scheme); - if (!sig.VerifyInsecure(pubKey, GetSignatureHash())) { + if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) { LogPrintf("CGovernanceVote::CheckSignature -- VerifyInsecure() failed\n"); return false; }