From 627e27c0218a4d3fed6118bbf287b909675933df Mon Sep 17 00:00:00 2001 From: echennells Date: Wed, 1 Jul 2026 16:52:07 +0000 Subject: [PATCH] Serialize node block in the requested form, stripping witness. A peer requesting a block without witness (getdata MSG_BLOCK) crashes the node. messages::block held the store's wire bytes plus an advisory witnessed_ flag, and serialize/size guarded on witness == witnessed_. The generic serializer calls serialize with the default witness = true, so the guard fails, serialize returns false, and the null payload is sent without a null check. Any peer can trigger it. The guard is the root error, but the witness argument must also function: a witness node servicing a non-witness peer must strip the witness on serialize, and the serializer cannot rely on the held object's form because one object may be sent to peers with differing requirements. Hold a system::chain::block_view instead of raw bytes and delegate serialize and size to block.to_data(witness) and block.serialized_size(witness); the view strips the witness when serialized without it. protocol_block_out_106 selects the form at read time via get_wire_block(link, witness) and wraps the result in the view. messages::transaction carries the same guard but nothing serializes it yet (transaction-out re-serializes from the parsed transaction), so it is left unchanged. Depends on the libbitcoin-system block_view and transaction_view to_data implementation. --- builds/gnu/Makefile.am | 1 + .../libbitcoin-node-test.vcxproj | 1 + .../libbitcoin-node-test.vcxproj.filters | 10 ++- .../libbitcoin-node-test.vcxproj | 1 + .../libbitcoin-node-test.vcxproj.filters | 10 ++- include/bitcoin/node/messages/block.hpp | 11 ++-- src/messages/block.cpp | 16 ++--- src/protocols/protocol_block_out_106.cpp | 7 +- test/messages/block.cpp | 64 +++++++++++++++++++ 9 files changed, 97 insertions(+), 24 deletions(-) create mode 100644 test/messages/block.cpp diff --git a/builds/gnu/Makefile.am b/builds/gnu/Makefile.am index 07610b0a..1925346a 100644 --- a/builds/gnu/Makefile.am +++ b/builds/gnu/Makefile.am @@ -238,6 +238,7 @@ test_libbitcoin_node_test_SOURCES = \ ${srcdir}/../../test/chasers/chaser_template.cpp \ ${srcdir}/../../test/chasers/chaser_transaction.cpp \ ${srcdir}/../../test/chasers/chaser_validate.cpp \ + ${srcdir}/../../test/messages/block.cpp \ ${srcdir}/../../test/protocols/protocol.cpp \ ${srcdir}/../../test/sessions/session.cpp diff --git a/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj b/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj index de485540..5eaccba2 100644 --- a/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj +++ b/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj @@ -135,6 +135,7 @@ + diff --git a/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters b/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters index 3e9441ba..7a4755f8 100644 --- a/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters +++ b/builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters @@ -13,12 +13,15 @@ {4BD50864-D3BC-4F64-0000-000000000001} - + {4BD50864-D3BC-4F64-0000-000000000002} - + {4BD50864-D3BC-4F64-0000-000000000003} + + {4BD50864-D3BC-4F64-0000-000000000004} + @@ -72,6 +75,9 @@ src + + src\messages + src\protocols diff --git a/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj b/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj index 1ed68de9..2f4eb81c 100644 --- a/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj +++ b/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj @@ -135,6 +135,7 @@ + diff --git a/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters b/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters index 3e9441ba..7a4755f8 100644 --- a/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters +++ b/builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters @@ -13,12 +13,15 @@ {4BD50864-D3BC-4F64-0000-000000000001} - + {4BD50864-D3BC-4F64-0000-000000000002} - + {4BD50864-D3BC-4F64-0000-000000000003} + + {4BD50864-D3BC-4F64-0000-000000000004} + @@ -72,6 +75,9 @@ src + + src\messages + src\protocols diff --git a/include/bitcoin/node/messages/block.hpp b/include/bitcoin/node/messages/block.hpp index 9e57a419..1b3577fc 100644 --- a/include/bitcoin/node/messages/block.hpp +++ b/include/bitcoin/node/messages/block.hpp @@ -41,18 +41,17 @@ struct BCN_API block ////static block deserialize(uint32_t version, system::reader& source, //// bool witness=true) NOEXCEPT; - /// These return false if witness or version is inconsistent with block data. + /// The held block is serialized in the requested form; a witnessed view + /// is stripped when serialized without witness. + /// The bool overload returns false only on a short output buffer. bool serialize(uint32_t version, const system::data_slab& data, bool witness=true) const NOEXCEPT; void serialize(uint32_t version, system::writer& sink, bool witness=true) const NOEXCEPT; size_t size(uint32_t version, bool witness=true) const NOEXCEPT; - /// Wire serialized block. - system::data_chunk block_data{}; - - /// Block contains witness data (if applicable). - const bool witnessed_{}; + /// The block, serialized on demand as witnessed or stripped. + system::chain::block_view block; }; } // namespace messages diff --git a/src/messages/block.cpp b/src/messages/block.cpp index 9409442e..b79a89ec 100644 --- a/src/messages/block.cpp +++ b/src/messages/block.cpp @@ -36,28 +36,20 @@ const uint32_t block::version_maximum = level::maximum_protocol; bool block::serialize(uint32_t version, const data_slab& data, bool witness) const NOEXCEPT { - if (witness != witnessed_) - return false; - system::stream::out::fast out{ data }; system::write::bytes::fast writer{ out }; serialize(version, writer, witness); return writer; } -// Sender must ensure that version/witness are consistent with channel. -void block::serialize(uint32_t, writer& sink, - bool BC_DEBUG_ONLY(witness)) const NOEXCEPT +void block::serialize(uint32_t, writer& sink, bool witness) const NOEXCEPT { - BC_ASSERT(witness == witnessed_); - sink.write_bytes(block_data); + block.to_data(sink, witness); } -// Sender must ensure that version/witness are consistent with channel. -size_t block::size(uint32_t, bool BC_DEBUG_ONLY(witness)) const NOEXCEPT +size_t block::size(uint32_t, bool witness) const NOEXCEPT { - BC_ASSERT(witness == witnessed_); - return block_data.size(); + return block.is_valid() ? block.serialized_size(witness) : zero; } } // namespace messages diff --git a/src/protocols/protocol_block_out_106.cpp b/src/protocols/protocol_block_out_106.cpp index c59fca93..be8f6630 100644 --- a/src/protocols/protocol_block_out_106.cpp +++ b/src/protocols/protocol_block_out_106.cpp @@ -229,8 +229,11 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT } const auto start = logger::now(); - node::messages::block out{ query.get_wire_block(link, witness), witness }; - if (out.block_data.empty()) + node::messages::block out + { + { query.get_wire_block(link, witness), witness } + }; + if (!out.block.is_valid()) { LOGR("Requested block " << encode_hash(item.hash) << " from [" << opposite() << "] not found."); diff --git a/test/messages/block.cpp b/test/messages/block.cpp new file mode 100644 index 00000000..4ae27b0e --- /dev/null +++ b/test/messages/block.cpp @@ -0,0 +1,64 @@ +/** + * Copyright (c) 2011-2026 libbitcoin developers (see AUTHORS) + * + * This file is part of libbitcoin. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +#include "../test.hpp" + +BOOST_AUTO_TEST_SUITE(block_tests) + +using namespace network::messages; + +// The message holds a block_view and serializes in the requested form. Genesis +// is non-witness (witnessed and stripped forms are identical); the witnessed +// strip is covered by the libbitcoin-system block_view to_data tests. + +BOOST_AUTO_TEST_CASE(block__serialize__witness__expected) +{ + const system::settings settings{ system::chain::selection::mainnet }; + const node::messages::block instance + { + { settings.genesis_block.to_data(true), true } + }; + system::data_chunk buffer(instance.size(peer::level::canonical, true)); + BOOST_REQUIRE(instance.serialize(peer::level::canonical, { buffer }, true)); + BOOST_REQUIRE_EQUAL(buffer, settings.genesis_block.to_data(true)); +} + +BOOST_AUTO_TEST_CASE(block__serialize__non_witness__expected) +{ + const system::settings settings{ system::chain::selection::mainnet }; + const node::messages::block instance + { + { settings.genesis_block.to_data(true), false } + }; + system::data_chunk buffer(instance.size(peer::level::canonical, false)); + BOOST_REQUIRE(instance.serialize(peer::level::canonical, { buffer }, false)); + BOOST_REQUIRE_EQUAL(buffer, settings.genesis_block.to_data(false)); +} + +BOOST_AUTO_TEST_CASE(block__serialize__short_buffer__false) +{ + const system::settings settings{ system::chain::selection::mainnet }; + const node::messages::block instance + { + { settings.genesis_block.to_data(true), true } + }; + system::data_chunk buffer(sub1(instance.size(peer::level::canonical, true))); + BOOST_REQUIRE(!instance.serialize(peer::level::canonical, { buffer }, true)); +} + +BOOST_AUTO_TEST_SUITE_END()