maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #16021: p2p: Avoid logging transaction decode errors to stderr

Browse source 
fa2b52af32 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 #14039
  *  Add test for superfluous witness record in deserialization #15893

ACKs for commit fa2b52:
  laanwj:
    utACK fa2b52af32
  ryanofsky:
    utACK fa2b52af32. 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
This commit is contained in:
Wladimir J. van der Laan 2019-05-20 17:27:56 +02:00
parent dfc02dac0f fa2b52af32
commit bb291b50f2
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
2 changed files with 33 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
src/net_processing.cpp
View file

@ -3288,23 +3288,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(&e, "ProcessMessages()");
}
}

26
test/functional/p2p_segwit.py
View file

@ -2038,9 +2038,9 @@ class SegWitTest(BitcoinTestFramework):
# TODO: test p2sh sigop counting
def test_superfluous_witness(self):
# Serialization of tx that puts witness flag to 1 always
# Serialization of tx that puts witness flag to 3 always
def serialize_with_bogus_witness(tx):
flags = 1
flags = 3
r = b""
r += struct.pack("<i", tx.nVersion)
if flags:
@ -2059,9 +2059,29 @@ class SegWitTest(BitcoinTestFramework):
r += struct.pack("<I", tx.nLockTime)
return r
raw = self.nodes[0].createrawtransaction([{"txid":"00"*32, "vout":0}], {self.nodes[0].getnewaddress():1})
class msg_bogus_tx(msg_tx):
def serialize(self):
return serialize_with_bogus_witness(self.tx)
self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(address_type='bech32'), 5)
self.nodes[0].generate(1)
unspent = next(u for u in self.nodes[0].listunspent() if u['spendable'] and u['address'].startswith('bcrt'))
raw = self.nodes[0].createrawtransaction([{"txid": unspent['txid'], "vout": unspent['vout']}], {self.nodes[0].getnewaddress(): 1})
tx = FromHex(CTransaction(), raw)
assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].decoderawtransaction, serialize_with_bogus_witness(tx).hex())
with self.nodes[0].assert_debug_log(['Superfluous witness record']):
self.nodes[0].p2p.send_message(msg_bogus_tx(tx))
self.nodes[0].p2p.sync_with_ping()
raw = self.nodes[0].signrawtransactionwithwallet(raw)
assert raw['complete']
raw = raw['hex']
tx = FromHex(CTransaction(), raw)
assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].decoderawtransaction, serialize_with_bogus_witness(tx).hex())
with self.nodes[0].assert_debug_log(['Unknown transaction optional data']):
self.nodes[0].p2p.send_message(msg_bogus_tx(tx))
self.nodes[0].p2p.sync_with_ping()
if __name__ == '__main__':
SegWitTest().main()