From fcd3b4fd4999bc2a995944f0b71ea57d24eb8468 Mon Sep 17 00:00:00 2001 From: Nathan Marley Date: Tue, 26 Feb 2019 07:44:43 -0300 Subject: [PATCH] Disallow new proposals using legacy serialization (#2722) * add flag to allow legacy proposal format * add proposal validator ctor flag for legacy format * add test for legacy proposal format disabled --- src/governance-object.cpp | 2 +- src/governance-validators.cpp | 13 +++++++++---- src/governance-validators.h | 3 ++- src/governance.cpp | 2 +- src/rpc/governance.cpp | 6 +++--- src/test/governance_validators_tests.cpp | 13 +++++++++---- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/governance-object.cpp b/src/governance-object.cpp index 2593846b2..902138fd4 100644 --- a/src/governance-object.cpp +++ b/src/governance-object.cpp @@ -469,7 +469,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingMast return false; } case GOVERNANCE_OBJECT_PROPOSAL: { - CProposalValidator validator(GetDataAsHexString()); + CProposalValidator validator(GetDataAsHexString(), true); // Note: It's ok to have expired proposals // they are going to be cleared by CGovernanceManager::UpdateCachesAndClean() // TODO: should they be tagged as "expired" to skip vote downloading? diff --git a/src/governance-validators.cpp b/src/governance-validators.cpp index 3e9a36a1a..eccc8def6 100644 --- a/src/governance-validators.cpp +++ b/src/governance-validators.cpp @@ -14,9 +14,10 @@ const size_t MAX_DATA_SIZE = 512; const size_t MAX_NAME_SIZE = 40; -CProposalValidator::CProposalValidator(const std::string& strHexData) : +CProposalValidator::CProposalValidator(const std::string& strHexData, bool fAllowLegacyFormat) : objJSON(UniValue::VOBJ), fJSONValid(false), + fAllowLegacyFormat(fAllowLegacyFormat), strErrorMessages() { if (!strHexData.empty()) { @@ -207,9 +208,13 @@ void CProposalValidator::ParseJSONData(const std::string& strJSONData) if (obj.isObject()) { objJSON = obj; } else { - std::vector arr1 = obj.getValues(); - std::vector arr2 = arr1.at(0).getValues(); - objJSON = arr2.at(1); + if (fAllowLegacyFormat) { + std::vector arr1 = obj.getValues(); + std::vector arr2 = arr1.at(0).getValues(); + objJSON = arr2.at(1); + } else { + throw std::runtime_error("Legacy proposal serialization format not allowed"); + } } fJSONValid = true; diff --git a/src/governance-validators.h b/src/governance-validators.h index e9de02ae1..3d78a69f0 100644 --- a/src/governance-validators.h +++ b/src/governance-validators.h @@ -14,10 +14,11 @@ class CProposalValidator private: UniValue objJSON; bool fJSONValid; + bool fAllowLegacyFormat; std::string strErrorMessages; public: - CProposalValidator(const std::string& strDataHexIn = std::string()); + CProposalValidator(const std::string& strDataHexIn = std::string(), bool fAllowLegacyFormat = true); bool Validate(bool fCheckExpiration = true); diff --git a/src/governance.cpp b/src/governance.cpp index f59391d8d..898b97eaa 100644 --- a/src/governance.cpp +++ b/src/governance.cpp @@ -452,7 +452,7 @@ void CGovernanceManager::UpdateCachesAndClean() } else { // NOTE: triggers are handled via triggerman if (pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { - CProposalValidator validator(pObj->GetDataAsHexString()); + CProposalValidator validator(pObj->GetDataAsHexString(), true); if (!validator.Validate()) { LogPrintf("CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", (*it).first.ToString()); pObj->fCachedDelete = true; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 4d7e4006c..eb59f3d7a 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -102,7 +102,7 @@ UniValue gobject_check(const JSONRPCRequest& request) CGovernanceObject govobj(hashParent, nRevision, nTime, uint256(), strDataHex); if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { - CProposalValidator validator(strDataHex); + CProposalValidator validator(strDataHex, false); if (!validator.Validate()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages()); } @@ -176,7 +176,7 @@ UniValue gobject_prepare(const JSONRPCRequest& request) govobj.GetDataAsPlainString(), govobj.GetHash().ToString()); if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { - CProposalValidator validator(strDataHex); + CProposalValidator validator(strDataHex, false); if (!validator.Validate()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages()); } @@ -297,7 +297,7 @@ UniValue gobject_submit(const JSONRPCRequest& request) << std::endl; ); if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { - CProposalValidator validator(strDataHex); + CProposalValidator validator(strDataHex, false); if (!validator.Validate()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages()); } diff --git a/src/test/governance_validators_tests.cpp b/src/test/governance_validators_tests.cpp index c45022602..1d2e0c8dd 100644 --- a/src/test/governance_validators_tests.cpp +++ b/src/test/governance_validators_tests.cpp @@ -45,13 +45,18 @@ BOOST_AUTO_TEST_CASE(valid_proposals_test) // legacy format std::string strHexData1 = CreateEncodedProposalObject(objProposal); - CProposalValidator validator1(strHexData1); + CProposalValidator validator1(strHexData1, true); BOOST_CHECK_MESSAGE(validator1.Validate(false), validator1.GetErrorMessages()); BOOST_CHECK_MESSAGE(!validator1.Validate(), validator1.GetErrorMessages()); + // legacy format w/validation flag off + CProposalValidator validator0(strHexData1, false); + BOOST_CHECK(!validator0.Validate()); + BOOST_CHECK_EQUAL(validator0.GetErrorMessages(), "Legacy proposal serialization format not allowed;JSON parsing error;"); + // new format std::string strHexData2 = HexStr(objProposal.write()); - CProposalValidator validator2(strHexData2); + CProposalValidator validator2(strHexData2, false); BOOST_CHECK_MESSAGE(validator2.Validate(false), validator2.GetErrorMessages()); BOOST_CHECK_MESSAGE(!validator2.Validate(), validator2.GetErrorMessages()); } @@ -69,12 +74,12 @@ BOOST_AUTO_TEST_CASE(invalid_proposals_test) // legacy format std::string strHexData1 = CreateEncodedProposalObject(objProposal); - CProposalValidator validator1(strHexData1); + CProposalValidator validator1(strHexData1, true); BOOST_CHECK_MESSAGE(!validator1.Validate(false), validator1.GetErrorMessages()); // new format std::string strHexData2 = HexStr(objProposal.write()); - CProposalValidator validator2(strHexData2); + CProposalValidator validator2(strHexData2, false); BOOST_CHECK_MESSAGE(!validator2.Validate(false), validator2.GetErrorMessages()); } }