Skip to content

Commit

Permalink
Disallow new proposals using legacy serialization (#2722)
Browse files Browse the repository at this point in the history
*  add flag to allow legacy proposal format

* add proposal validator ctor flag for legacy format

* add test for legacy proposal format disabled
  • Loading branch information
nmarley authored and UdjinM6 committed Feb 26, 2019
1 parent 668b84b commit fcd3b4f
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/governance-object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
13 changes: 9 additions & 4 deletions src/governance-validators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -207,9 +208,13 @@ void CProposalValidator::ParseJSONData(const std::string& strJSONData)
if (obj.isObject()) {
objJSON = obj;
} else {
std::vector<UniValue> arr1 = obj.getValues();
std::vector<UniValue> arr2 = arr1.at(0).getValues();
objJSON = arr2.at(1);
if (fAllowLegacyFormat) {
std::vector<UniValue> arr1 = obj.getValues();
std::vector<UniValue> arr2 = arr1.at(0).getValues();
objJSON = arr2.at(1);
} else {
throw std::runtime_error("Legacy proposal serialization format not allowed");
}
}

fJSONValid = true;
Expand Down
3 changes: 2 additions & 1 deletion src/governance-validators.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/rpc/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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());
}
Expand Down
13 changes: 9 additions & 4 deletions src/test/governance_validators_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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());
}
}
Expand Down

0 comments on commit fcd3b4f

Please sign in to comment.