sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix node protection logic false positives (#3314)

Browse Source 
We could be reading multiple messages from a socket buffer at once _without actually processing them yet_ which means that `fSuccessfullyConnected` might not be switched to `true` at the time we already parsed `VERACK` message and started to parse the next one. This is basically a false positive and we drop a legit node as a result even though the order of messages sent by this node was completely fine. To fix this I partially reverted #2790 (where the issue was initially introduced) and moved the logic for tracking the first message into ProcessMessage instead.
This commit is contained in:
UdjinM6 2020-02-01 04:59:01 +03:00
parent 8d5fc6e0ab
commit c0a671e840
2 changed files with 8 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/net.cpp
View File

@ -744,24 +744,6 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete
int handled;
if (!msg.in_data) {
handled = msg.readHeader(pch, nBytes);
if (msg.in_data && nTimeFirstMessageReceived == 0) {
if (fSuccessfullyConnected) {
// First message after VERSION/VERACK.
// We record the time when the header is fully received and not when the full message is received.
// otherwise a peer might send us a very large message as first message after VERSION/VERACK and fill
// up our memory with multiple parallel connections doing this.
nTimeFirstMessageReceived = nTimeMicros;
fFirstMessageIsMNAUTH = msg.hdr.GetCommand() == NetMsgType::MNAUTH;
} else {
// We're still in the VERSION/VERACK handshake process, so any message received in this state is
// expected to be very small. This protects against attackers filling up memory by sending oversized
// VERSION messages while the incoming connection is still protected against eviction
if (msg.hdr.nMessageSize > 1024) {
LogPrint(BCLog::NET, "Oversized VERSION/VERACK message from peer=%i, disconnecting\n", GetId());
return false;
}
}
}
} else {
handled = msg.readData(pch, nBytes);
}
@ -1008,7 +990,7 @@ bool CConnman::AttemptToEvictConnection()
if (fMasternodeMode) {
// This handles eviction protected nodes. Nodes are always protected for a short time after the connection
// was accepted. This short time is meant for the VERSION/VERACK exchange and the possible MNAUTH that might
// follow when the incoming connection is from another masternode. When another message then MNAUTH
// follow when the incoming connection is from another masternode. When a message other than MNAUTH
// is received after VERSION/VERACK, the protection is lifted immediately.
bool isProtected = GetSystemTimeInSeconds() - node->nTimeConnected < INBOUND_EVICTION_PROTECTION_TIME;
if (node->nTimeFirstMessageReceived != 0 && !node->fFirstMessageIsMNAUTH) {

7
src/net_processing.cpp
View File

@ -2037,6 +2037,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return false;
}
if (pfrom->nTimeFirstMessageReceived == 0) {
// First message after VERSION/VERACK
pfrom->nTimeFirstMessageReceived = GetTimeMicros();
pfrom->fFirstMessageIsMNAUTH = strCommand == NetMsgType::MNAUTH;
// Note: do not break the flow here
}
if (strCommand == NetMsgType::ADDR) {
std::vector<CAddress> vAddr;
vRecv >> vAddr;