Skip to content

Commit

Permalink
lock cs_main for Misbehaving
Browse files Browse the repository at this point in the history 
Github-Pull: PIVX-Project#1785
Rebased-From: bc04b4d
  • Loading branch information
@furszy @Fuzzbawls
furszy authored and Fuzzbawls committed Aug 12, 2020
1 parent 5c629d8 commit 5c3caa4
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 16 deletions.
14 changes: 9 additions & 5 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip)
}

// Requires cs_main.
void Misbehaving(NodeId pnode, int howmuch)
void Misbehaving(NodeId pnode, int howmuch) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
if (howmuch == 0)
return;
Expand Down Expand Up @@ -1687,8 +1687,10 @@ void static InvalidBlockFound(CBlockIndex* pindex, const CValidationState& state
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
CBlockReject reject = {(unsigned char) state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), pindex->GetBlockHash()};
State(it->second)->rejects.push_back(reject);
if (nDoS > 0)
if (nDoS > 0) {
LOCK(cs_main);
Misbehaving(it->second, nDoS);
}
}
}
if (!state.CorruptionPossible()) {
Expand Down Expand Up @@ -5214,9 +5216,11 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR

pfrom->fClient = !(pfrom->nServices & NODE_NETWORK);

// Potentially mark this peer as a preferred download peer.
UpdatePreferredDownload(pfrom, State(pfrom->GetId()));

{
LOCK(cs_main);
// Potentially mark this peer as a preferred download peer.
UpdatePreferredDownload(pfrom, State(pfrom->GetId()));
}
// Change version
pfrom->PushMessage(NetMsgType::VERACK);
pfrom->ssSend.SetVersion(std::min(pfrom->nVersion, PROTOCOL_VERSION));
Expand Down
2 changes: 1 addition & 1 deletion src/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ CBlockIndex* InsertBlockIndex(uint256 hash);
/** Get statistics from node state */
bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats);
/** Increase a node's misbehavior score. */
void Misbehaving(NodeId nodeid, int howmuch);
void Misbehaving(NodeId nodeid, int howmuch) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Flush all state, indexes and buffers to disk. */
void FlushStateToDisk();

Expand Down
3 changes: 3 additions & 0 deletions src/masternode-budget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,7 @@ void CBudgetManager::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
if (nProp.IsNull()) {
if (pfrom->HasFulfilledRequest("budgetvotesync")) {
LogPrint(BCLog::MNBUDGET,"mnvs - peer already asked me for the list\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 20);
return;
}
Expand Down Expand Up @@ -1149,6 +1150,7 @@ void CBudgetManager::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
if (!vote.CheckSignature()) {
if (masternodeSync.IsSynced()) {
LogPrintf("CBudgetManager::ProcessMessage() : mvote - signature invalid\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 20);
}
// it could just be a non-synced masternode
Expand Down Expand Up @@ -1223,6 +1225,7 @@ void CBudgetManager::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
if (!vote.CheckSignature()) {
if (masternodeSync.IsSynced()) {
LogPrintf("CBudgetManager::ProcessMessage() : fbvote - signature from masternode %s invalid\n", HexStr(pmn->pubKeyMasternode));
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 20);
}
// it could just be a non-synced masternode
Expand Down
2 changes: 2 additions & 0 deletions src/masternode-payments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ void CMasternodePayments::ProcessMessageMasternodePayments(CNode* pfrom, std::st
if (Params().NetworkID() == CBaseChainParams::MAIN) {
if (pfrom->HasFulfilledRequest(NetMsgType::GETMNWINNERS)) {
LogPrintf("CMasternodePayments::ProcessMessageMasternodePayments() : mnget - peer already asked me for the list\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 20);
return;
}
Expand Down Expand Up @@ -484,6 +485,7 @@ void CMasternodePayments::ProcessMessageMasternodePayments(CNode* pfrom, std::st
if (!winner.CheckSignature()) {
if (masternodeSync.IsSynced()) {
LogPrintf("CMasternodePayments::ProcessMessageMasternodePayments() : mnw - invalid signature\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 20);
}
// it could just be a non-synced masternode
Expand Down
12 changes: 9 additions & 3 deletions src/masternodeman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,10 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CData

int nDoS = 0;
if (!mnb.CheckAndUpdate(nDoS)) {
if (nDoS > 0)
if (nDoS > 0) {
LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS);

}
//failed
return;
}
Expand All @@ -711,6 +712,7 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
// - this is expensive, so it's only done once per Masternode
if (!mnb.IsInputAssociatedWithPubkey()) {
LogPrintf("CMasternodeMan::ProcessMessage() : mnb - Got mismatched pubkey and vin\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 33);
return;
}
Expand All @@ -724,8 +726,10 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
} else {
LogPrint(BCLog::MASTERNODE,"mnb - Rejected Masternode entry %s\n", mnb.vin.prevout.hash.ToString());

if (nDoS > 0)
if (nDoS > 0) {
LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS);
}
}
}

Expand All @@ -743,6 +747,7 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CData

if (nDoS > 0) {
// if anything significant failed, mark that node
LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS);
} else {
// if nothing significant failed, search existing Masternode list
Expand Down Expand Up @@ -770,6 +775,7 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CData
int64_t t = (*i).second;
if (GetTime() < t) {
LogPrintf("CMasternodeMan::ProcessMessage() : dseg - peer already asked me for the list\n");
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 34);
return;
}
Expand Down
19 changes: 12 additions & 7 deletions src/test/DoS_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,30 @@ CService ip(uint32_t i)

BOOST_FIXTURE_TEST_SUITE(DoS_tests, TestingSetup)

void misbehave(NodeId id, int value) {
LOCK(cs_main);
Misbehaving(id, value); // Should get banned
}

BOOST_AUTO_TEST_CASE(DoS_banning)
{
CNode::ClearBanned();
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
dummyNode1.nVersion = 1;
Misbehaving(dummyNode1.GetId(), 100); // Should get banned
misbehave(dummyNode1.GetId(), 100); // Should get banned
SendMessages(&dummyNode1);
BOOST_CHECK(CNode::IsBanned(addr1));
BOOST_CHECK(!CNode::IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned

CAddress addr2(ip(0xa0b0c002), NODE_NONE);
CNode dummyNode2(INVALID_SOCKET, addr2, "", true);
dummyNode2.nVersion = 1;
Misbehaving(dummyNode2.GetId(), 50);
misbehave(dummyNode2.GetId(), 50);
SendMessages(&dummyNode2);
BOOST_CHECK(!CNode::IsBanned(addr2)); // 2 not banned yet...
BOOST_CHECK(CNode::IsBanned(addr1)); // ... but 1 still should be
Misbehaving(dummyNode2.GetId(), 50);
misbehave(dummyNode2.GetId(), 50);
SendMessages(&dummyNode2);
BOOST_CHECK(CNode::IsBanned(addr2));
}
Expand All @@ -75,13 +80,13 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
dummyNode1.nVersion = 1;
Misbehaving(dummyNode1.GetId(), 100);
misbehave(dummyNode1.GetId(), 100);
SendMessages(&dummyNode1);
BOOST_CHECK(!CNode::IsBanned(addr1));
Misbehaving(dummyNode1.GetId(), 10);
misbehave(dummyNode1.GetId(), 10);
SendMessages(&dummyNode1);
BOOST_CHECK(!CNode::IsBanned(addr1));
Misbehaving(dummyNode1.GetId(), 1);
misbehave(dummyNode1.GetId(), 1);
SendMessages(&dummyNode1);
BOOST_CHECK(CNode::IsBanned(addr1));
mapArgs.erase("-banscore");
Expand All @@ -97,7 +102,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
CNode dummyNode(INVALID_SOCKET, addr, "", true);
dummyNode.nVersion = 1;

Misbehaving(dummyNode.GetId(), 100);
misbehave(dummyNode.GetId(), 100);
SendMessages(&dummyNode);
BOOST_CHECK(CNode::IsBanned(addr));

Expand Down

0 comments on commit 5c3caa4

Please sign in to comment.