Skip to content

Commit

Permalink
Merge bitcoin#16021: p2p: Avoid logging transaction decode errors to …
Browse files Browse the repository at this point in the history
…stderr

fa2b52a Disallow extended encoding for non-witness transactions (take 3) (MarcoFalke)

Pull request description:

  (previous title "p2p: Disallow extended encoding for non-witness transactions (take 3)")

  Remote peers can send us illegally encoded txs and thus have us write to stderr. Fix that by not writing to stderr.

  This is a follow up to the previous (incomplete) attempts at this:

  *  Disallow extended encoding for non-witness transactions bitcoin#14039
  *  Add test for superfluous witness record in deserialization bitcoin#15893

ACKs for commit fa2b52:
  laanwj:
    utACK fa2b52a
  ryanofsky:
    utACK fa2b52a. Would change title to something like "Avoid logging transaction decode errors to stderr" instead of "Disallow extended encoding for non-witness transactions." The current title is confusing because this PR isn't really allowing or disallowing anything, just logging the condition differently. "Disallow" also seems to contradict the "Allow exceptions from..." comments in the actual code.

Tree-SHA512: c66990e69b432d00dc1c5510bf976a1188664d0890a32d1e5c6459094e7e27da82a5d227627afcbc203676f5540eec74b7d9b1d71d2c62d3b2069e1781824b4d
  • Loading branch information
laanwj authored and Munkybooty committed Nov 12, 2021
1 parent d3e0c26 commit 740f21e
Showing 1 changed file with 10 additions and 11 deletions.
21 changes: 10 additions & 11 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4075,23 +4075,22 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
if (m_enable_bip61) {
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message")));
}
if (strstr(e.what(), "end of data"))
{
if (strstr(e.what(), "end of data")) {
// Allow exceptions from under-length message on vRecv
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
}
else if (strstr(e.what(), "size too large"))
{
} else if (strstr(e.what(), "size too large")) {
// Allow exceptions from over-long size
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
}
else if (strstr(e.what(), "non-canonical ReadCompactSize()"))
{
} else if (strstr(e.what(), "non-canonical ReadCompactSize()")) {
// Allow exceptions from non-canonical encoding
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
}
else
{
} else if (strstr(e.what(), "Superfluous witness record")) {
// Allow exceptions from illegal witness encoding
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
} else if (strstr(e.what(), "Unknown transaction optional data")) {
// Allow exceptions from unknown witness encoding
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
} else {
PrintExceptionContinue(std::current_exception(), "ProcessMessages()");
}
} catch (...) {
Expand Down

0 comments on commit 740f21e

Please sign in to comment.