Skip to content

Commit 9bde3f0

Browse files
committed
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.
1 parent ada11dc commit 9bde3f0

9 files changed

Lines changed: 97 additions & 24 deletions

File tree

‎builds/gnu/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ test_libbitcoin_node_test_SOURCES = \
238238
${srcdir}/../../test/chasers/chaser_template.cpp \
239239
${srcdir}/../../test/chasers/chaser_transaction.cpp \
240240
${srcdir}/../../test/chasers/chaser_validate.cpp \
241+
${srcdir}/../../test/messages/block.cpp \
241242
${srcdir}/../../test/protocols/protocol.cpp \
242243
${srcdir}/../../test/sessions/session.cpp
243244

‎builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@
135135
<ClCompile Include="..\..\..\..\test\estimator.cpp" />
136136
<ClCompile Include="..\..\..\..\test\full_node.cpp" />
137137
<ClCompile Include="..\..\..\..\test\main.cpp" />
138+
<ClCompile Include="..\..\..\..\test\messages\block.cpp" />
138139
<ClCompile Include="..\..\..\..\test\protocols\protocol.cpp" />
139140
<ClCompile Include="..\..\..\..\test\sessions\session.cpp" />
140141
<ClCompile Include="..\..\..\..\test\settings.cpp" />

‎builds/msvc/vs2022/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313
<Filter Include="src\chasers">
1414
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000001}</UniqueIdentifier>
1515
</Filter>
16-
<Filter Include="src\protocols">
16+
<Filter Include="src\messages">
1717
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000002}</UniqueIdentifier>
1818
</Filter>
19-
<Filter Include="src\sessions">
19+
<Filter Include="src\protocols">
2020
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000003}</UniqueIdentifier>
2121
</Filter>
22+
<Filter Include="src\sessions">
23+
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000004}</UniqueIdentifier>
24+
</Filter>
2225
</ItemGroup>
2326
<ItemGroup>
2427
<ClCompile Include="..\..\..\..\test\block_arena.cpp">
@@ -72,6 +75,9 @@
7275
<ClCompile Include="..\..\..\..\test\main.cpp">
7376
<Filter>src</Filter>
7477
</ClCompile>
78+
<ClCompile Include="..\..\..\..\test\messages\block.cpp">
79+
<Filter>src\messages</Filter>
80+
</ClCompile>
7581
<ClCompile Include="..\..\..\..\test\protocols\protocol.cpp">
7682
<Filter>src\protocols</Filter>
7783
</ClCompile>

‎builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@
135135
<ClCompile Include="..\..\..\..\test\estimator.cpp" />
136136
<ClCompile Include="..\..\..\..\test\full_node.cpp" />
137137
<ClCompile Include="..\..\..\..\test\main.cpp" />
138+
<ClCompile Include="..\..\..\..\test\messages\block.cpp" />
138139
<ClCompile Include="..\..\..\..\test\protocols\protocol.cpp" />
139140
<ClCompile Include="..\..\..\..\test\sessions\session.cpp" />
140141
<ClCompile Include="..\..\..\..\test\settings.cpp" />

‎builds/msvc/vs2026/libbitcoin-node-test/libbitcoin-node-test.vcxproj.filters‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313
<Filter Include="src\chasers">
1414
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000001}</UniqueIdentifier>
1515
</Filter>
16-
<Filter Include="src\protocols">
16+
<Filter Include="src\messages">
1717
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000002}</UniqueIdentifier>
1818
</Filter>
19-
<Filter Include="src\sessions">
19+
<Filter Include="src\protocols">
2020
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000003}</UniqueIdentifier>
2121
</Filter>
22+
<Filter Include="src\sessions">
23+
<UniqueIdentifier>{4BD50864-D3BC-4F64-0000-000000000004}</UniqueIdentifier>
24+
</Filter>
2225
</ItemGroup>
2326
<ItemGroup>
2427
<ClCompile Include="..\..\..\..\test\block_arena.cpp">
@@ -72,6 +75,9 @@
7275
<ClCompile Include="..\..\..\..\test\main.cpp">
7376
<Filter>src</Filter>
7477
</ClCompile>
78+
<ClCompile Include="..\..\..\..\test\messages\block.cpp">
79+
<Filter>src\messages</Filter>
80+
</ClCompile>
7581
<ClCompile Include="..\..\..\..\test\protocols\protocol.cpp">
7682
<Filter>src\protocols</Filter>
7783
</ClCompile>

‎include/bitcoin/node/messages/block.hpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,17 @@ struct BCN_API block
4141
////static block deserialize(uint32_t version, system::reader& source,
4242
//// bool witness=true) NOEXCEPT;
4343

44-
/// These return false if witness or version is inconsistent with block data.
44+
/// The held block is serialized in the requested form; a witnessed view
45+
/// is stripped when serialized without witness.
46+
/// The bool overload returns false only on a short output buffer.
4547
bool serialize(uint32_t version, const system::data_slab& data,
4648
bool witness=true) const NOEXCEPT;
4749
void serialize(uint32_t version, system::writer& sink,
4850
bool witness=true) const NOEXCEPT;
4951
size_t size(uint32_t version, bool witness=true) const NOEXCEPT;
5052

51-
/// Wire serialized block.
52-
system::data_chunk block_data{};
53-
54-
/// Block contains witness data (if applicable).
55-
const bool witnessed_{};
53+
/// The block, serialized on demand as witnessed or stripped.
54+
system::chain::block_view block;
5655
};
5756

5857
} // namespace messages

‎src/messages/block.cpp‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,20 @@ const uint32_t block::version_maximum = level::maximum_protocol;
3636
bool block::serialize(uint32_t version, const data_slab& data,
3737
bool witness) const NOEXCEPT
3838
{
39-
if (witness != witnessed_)
40-
return false;
41-
4239
system::stream::out::fast out{ data };
4340
system::write::bytes::fast writer{ out };
4441
serialize(version, writer, witness);
4542
return writer;
4643
}
4744

48-
// Sender must ensure that version/witness are consistent with channel.
49-
void block::serialize(uint32_t, writer& sink,
50-
bool BC_DEBUG_ONLY(witness)) const NOEXCEPT
45+
void block::serialize(uint32_t, writer& sink, bool witness) const NOEXCEPT
5146
{
52-
BC_ASSERT(witness == witnessed_);
53-
sink.write_bytes(block_data);
47+
block.to_data(sink, witness);
5448
}
5549

56-
// Sender must ensure that version/witness are consistent with channel.
57-
size_t block::size(uint32_t, bool BC_DEBUG_ONLY(witness)) const NOEXCEPT
50+
size_t block::size(uint32_t, bool witness) const NOEXCEPT
5851
{
59-
BC_ASSERT(witness == witnessed_);
60-
return block_data.size();
52+
return block.is_valid() ? block.serialized_size(witness) : zero;
6153
}
6254

6355
} // namespace messages

‎src/protocols/protocol_block_out_106.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,11 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT
226226
}
227227

228228
const auto start = logger::now();
229-
node::messages::block out{ query.get_wire_block(link, witness), witness };
230-
if (out.block_data.empty())
229+
node::messages::block out
230+
{
231+
{ query.get_wire_block(link, witness), witness }
232+
};
233+
if (!out.block.is_valid())
231234
{
232235
LOGR("Requested block " << encode_hash(item.hash) << " from ["
233236
<< opposite() << "] not found.");

‎test/messages/block.cpp‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* Copyright (c) 2011-2026 libbitcoin developers (see AUTHORS)
3+
*
4+
* This file is part of libbitcoin.
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU Affero General Public License as published by
8+
* the Free Software Foundation, either version 3 of the License, or
9+
* (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Affero General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Affero General Public License
17+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*/
19+
#include "../test.hpp"
20+
21+
BOOST_AUTO_TEST_SUITE(block_tests)
22+
23+
using namespace network::messages;
24+
25+
// The message holds a block_view and serializes in the requested form. Genesis
26+
// is non-witness (witnessed and stripped forms are identical); the witnessed
27+
// strip is covered by the libbitcoin-system block_view to_data tests.
28+
29+
BOOST_AUTO_TEST_CASE(block__serialize__witness__expected)
30+
{
31+
const system::settings settings{ system::chain::selection::mainnet };
32+
const node::messages::block instance
33+
{
34+
{ settings.genesis_block.to_data(true), true }
35+
};
36+
system::data_chunk buffer(instance.size(peer::level::canonical, true));
37+
BOOST_REQUIRE(instance.serialize(peer::level::canonical, { buffer }, true));
38+
BOOST_REQUIRE_EQUAL(buffer, settings.genesis_block.to_data(true));
39+
}
40+
41+
BOOST_AUTO_TEST_CASE(block__serialize__non_witness__expected)
42+
{
43+
const system::settings settings{ system::chain::selection::mainnet };
44+
const node::messages::block instance
45+
{
46+
{ settings.genesis_block.to_data(true), false }
47+
};
48+
system::data_chunk buffer(instance.size(peer::level::canonical, false));
49+
BOOST_REQUIRE(instance.serialize(peer::level::canonical, { buffer }, false));
50+
BOOST_REQUIRE_EQUAL(buffer, settings.genesis_block.to_data(false));
51+
}
52+
53+
BOOST_AUTO_TEST_CASE(block__serialize__short_buffer__false)
54+
{
55+
const system::settings settings{ system::chain::selection::mainnet };
56+
const node::messages::block instance
57+
{
58+
{ settings.genesis_block.to_data(true), true }
59+
};
60+
system::data_chunk buffer(sub1(instance.size(peer::level::canonical, true)));
61+
BOOST_REQUIRE(!instance.serialize(peer::level::canonical, { buffer }, true));
62+
}
63+
64+
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)