diff --git a/src/net.h b/src/net.h --- a/src/net.h +++ b/src/net.h @@ -58,11 +58,6 @@ static const unsigned int MAX_INV_SZ = 50000; /** The maximum number of new addresses to accumulate before announcing. */ static const unsigned int MAX_ADDR_TO_SEND = 1000; -/** - * Maximum length of incoming protocol messages (Currently 1MB). - * NB: Messages propagating block content are not subject to this limit. - */ -static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 1 * 1024 * 1024; /** Maximum length of strSubVer in `version` message */ static const unsigned int MAX_SUBVERSION_LENGTH = 256; /** Maximum number of automatic outgoing nodes */ @@ -572,7 +567,7 @@ vRecv.SetVersion(nVersionIn); } - int readHeader(const char *pch, unsigned int nBytes); + int readHeader(const Config &config, const char *pch, unsigned int nBytes); int readData(const char *pch, unsigned int nBytes); }; diff --git a/src/net.cpp b/src/net.cpp --- a/src/net.cpp +++ b/src/net.cpp @@ -704,19 +704,7 @@ return false; } - // If the message doesn't not contain a block content, check against - // MAX_PROTOCOL_MESSAGE_LENGTH. - if (msg.hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH && - !NetMsgType::IsBlockLike(msg.hdr.GetCommand())) { - return true; - } - - // Scale the maximum accepted size with the block size. - if (msg.hdr.nMessageSize > 2 * config.GetMaxBlockSize()) { - return true; - } - - return false; + return msg.hdr.IsOversized(config); } bool CNode::ReceiveMsgBytes(const Config &config, const char *pch, @@ -738,7 +726,7 @@ // Absorb network data. int handled; if (!msg.in_data) { - handled = msg.readHeader(pch, nBytes); + handled = msg.readHeader(config, pch, nBytes); } else { handled = msg.readData(pch, nBytes); } @@ -803,7 +791,8 @@ return nSendVersion; } -int CNetMessage::readHeader(const char *pch, unsigned int nBytes) { +int CNetMessage::readHeader(const Config &config, const char *pch, + unsigned int nBytes) { // copy data to temporary parsing buffer unsigned int nRemaining = 24 - nHdrPos; unsigned int nCopy = std::min(nRemaining, nBytes); @@ -823,8 +812,9 @@ return -1; } - // reject messages larger than MAX_SIZE - if (hdr.nMessageSize > MAX_SIZE) { + // Reject oversized messages + if (hdr.IsOversized(config)) { + LogPrint(BCLog::NET, "Oversized header detected\n"); return -1; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3139,7 +3139,7 @@ // Read header CMessageHeader &hdr = msg.hdr; - if (!hdr.IsValid(chainparams.NetMagic())) { + if (!hdr.IsValid(config)) { LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->id); return fMoreWork; diff --git a/src/protocol.h b/src/protocol.h --- a/src/protocol.h +++ b/src/protocol.h @@ -19,6 +19,14 @@ #include #include +class Config; + +/** + * Maximum length of incoming protocol messages (Currently 1MB). + * NB: Messages propagating block content are not subject to this limit. + */ +static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 1 * 1024 * 1024; + /** * Message header. * (4) message start. @@ -46,7 +54,9 @@ const char *pszCommand, unsigned int nMessageSizeIn); std::string GetCommand() const; - bool IsValid(const MessageMagic &messageStart) const; + bool IsValid(const Config &config) const; + bool IsValidWithoutConfig(const MessageMagic &magic) const; + bool IsOversized(const Config &config) const; ADD_SERIALIZE_METHODS; diff --git a/src/protocol.cpp b/src/protocol.cpp --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -5,6 +5,8 @@ #include "protocol.h" +#include "chainparams.h" +#include "config.h" #include "util.h" #include "utilstrencodings.h" @@ -90,18 +92,22 @@ pchCommand + strnlen(pchCommand, COMMAND_SIZE)); } -bool CMessageHeader::IsValid(const MessageMagic &pchMessageStartIn) const { +static bool +CheckHeaderMagicAndCommand(const CMessageHeader &header, + const CMessageHeader::MessageMagic &magic) { // Check start string - if (memcmp(std::begin(pchMessageStart), std::begin(pchMessageStartIn), - MESSAGE_START_SIZE) != 0) { + if (memcmp(std::begin(header.pchMessageStart), std::begin(magic), + CMessageHeader::MESSAGE_START_SIZE) != 0) { return false; } // Check the command string for errors - for (const char *p1 = pchCommand; p1 < pchCommand + COMMAND_SIZE; p1++) { + for (const char *p1 = header.pchCommand; + p1 < header.pchCommand + CMessageHeader::COMMAND_SIZE; p1++) { if (*p1 == 0) { // Must be all zeros after the first zero - for (; p1 < pchCommand + COMMAND_SIZE; p1++) { + for (; p1 < header.pchCommand + CMessageHeader::COMMAND_SIZE; + p1++) { if (*p1 != 0) { return false; } @@ -111,10 +117,19 @@ } } + return true; +} + +bool CMessageHeader::IsValid(const Config &config) const { + // Check start string + if (!CheckHeaderMagicAndCommand(*this, + config.GetChainParams().NetMagic())) { + return false; + } + // Message size - if (nMessageSize > MAX_SIZE) { - LogPrintf("CMessageHeader::IsValid(): (%s, %u bytes) nMessageSize > " - "MAX_SIZE\n", + if (IsOversized(config)) { + LogPrintf("CMessageHeader::IsValid(): (%s, %u bytes) is oversized\n", GetCommand(), nMessageSize); return false; } @@ -122,6 +137,46 @@ return true; } +/** + * This is a transition method in order to stay compatible with older code that + * do not use the config. It assumes message will not get too large. This cannot + * be used for any piece of code that will download blocks as blocks may be + * bigger than the permitted size. Idealy, code that uses this function should + * be migrated toward using the config. + */ +bool CMessageHeader::IsValidWithoutConfig(const MessageMagic &magic) const { + // Check start string + if (!CheckHeaderMagicAndCommand(*this, magic)) { + return false; + } + + // Message size + if (nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) { + LogPrintf( + "CMessageHeader::IsValidForSeeder(): (%s, %u bytes) is oversized\n", + GetCommand(), nMessageSize); + return false; + } + + return true; +} + +bool CMessageHeader::IsOversized(const Config &config) const { + // If the message doesn't not contain a block content, check against + // MAX_PROTOCOL_MESSAGE_LENGTH. + if (nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH && + !NetMsgType::IsBlockLike(GetCommand())) { + return true; + } + + // Scale the maximum accepted size with the block size. + if (nMessageSize > 2 * config.GetMaxBlockSize()) { + return true; + } + + return false; +} + CAddress::CAddress() : CService() { Init(); } diff --git a/src/seeder/bitcoin.cpp b/src/seeder/bitcoin.cpp --- a/src/seeder/bitcoin.cpp +++ b/src/seeder/bitcoin.cpp @@ -204,7 +204,7 @@ vRecv.begin() + nHeaderSize); CMessageHeader hdr(netMagic); vRecv >> hdr; - if (!hdr.IsValid(netMagic)) { + if (!hdr.IsValidWithoutConfig(netMagic)) { // printf("%s: BAD (invalid header)\n", ToString(you).c_str()); ban = 100000; return true; diff --git a/src/test/test_bitcoin_fuzzy.cpp b/src/test/test_bitcoin_fuzzy.cpp --- a/src/test/test_bitcoin_fuzzy.cpp +++ b/src/test/test_bitcoin_fuzzy.cpp @@ -197,7 +197,7 @@ try { CMessageHeader mh(pchMessageStart); ds >> mh; - if (!mh.IsValid(pchMessageStart)) { + if (!mh.IsValidWithoutConfig(pchMessageStart)) { return 0; } } catch (const std::ios_base::failure &e) { diff --git a/test/functional/abc-p2p-fullblocktest.py b/test/functional/abc-p2p-fullblocktest.py --- a/test/functional/abc-p2p-fullblocktest.py +++ b/test/functional/abc-p2p-fullblocktest.py @@ -75,7 +75,7 @@ self.coinbase_pubkey = self.coinbase_key.get_pubkey() self.tip = None self.blocks = {} - self.excessive_block_size = 32 * ONE_MEGABYTE + self.excessive_block_size = 64 * ONE_MEGABYTE self.extra_args = [['-norelaypriority', '-whitelist=127.0.0.1', '-limitancestorcount=9999',