Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix node protection logic false positives #3314

Merged
merged 1 commit into from
Feb 1, 2020
Merged

Fix node protection logic false positives #3314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions src/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,24 +776,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);
}
Expand Down Expand Up @@ -1050,7 +1032,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) {
Expand Down
7 changes: 7 additions & 0 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,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;
Expand Down