From 23ae538a6d20a37dca088a32d8416b68ab83c655 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 24 Aug 2026 15:48:48 +0100 Subject: [PATCH 1/4] Validate tree deserialisation bounds Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- merklecpp.h | 43 ++++++++++++++++++++++++++++++++++++------- test/unit_tests.cpp | 20 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/merklecpp.h b/merklecpp.h index 0306f2e..3ef1705 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -93,6 +93,17 @@ namespace merkle return r; } + static inline size_t deserialise_size_t( + const std::vector& bytes, size_t& index) + { + const auto value = deserialise_uint64_t(bytes, index); + if (value > std::numeric_limits::max()) + { + throw std::runtime_error("serialised value exceeds platform limits"); + } + return static_cast(value); + } + static inline bool decode_hex_digit(char c, uint8_t& value) { if ('0' <= c && c <= '9') @@ -433,9 +444,9 @@ namespace merkle MERKLECPP_TRACE(MERKLECPP_TOUT << "> PathT::deserialise " << std::endl); elements.clear(); _leaf.deserialise(bytes, position); - _leaf_index = deserialise_uint64_t(bytes, position); - _max_index = deserialise_uint64_t(bytes, position); - size_t const num_elements = deserialise_uint64_t(bytes, position); + _leaf_index = deserialise_size_t(bytes, position); + _max_index = deserialise_size_t(bytes, position); + size_t const num_elements = deserialise_size_t(bytes, position); for (size_t i = 0; i < num_elements; i++) { HashT hash(bytes, position); @@ -610,6 +621,19 @@ namespace merkle return r; } + /// @brief Constructs a new tree node + /// @param hash The hash to move into the node + static Node* make(HashT&& hash) + { + auto r = new Node(); + r->left = r->right = nullptr; + r->hash = std::move(hash); + r->dirty = false; + r->update_sizes(); + assert(r->invariant()); + return r; + } + /// @brief Constructs a new tree node /// @param left The left child of the new node /// @param right The right child of the new node @@ -1523,14 +1547,19 @@ namespace merkle clear(); - size_t num_leaf_nodes = deserialise_uint64_t(bytes, position); - num_flushed = deserialise_uint64_t(bytes, position); + const size_t num_leaf_nodes = deserialise_size_t(bytes, position); + num_flushed = deserialise_size_t(bytes, position); + if ( + position > bytes.size() || + num_leaf_nodes > (bytes.size() - position) / HASH_SIZE) + { + throw std::runtime_error("not enough bytes"); + } leaf_nodes.reserve(num_leaf_nodes); for (size_t i = 0; i < num_leaf_nodes; i++) { - Node* n = Node::make(bytes.data() + position); - position += HASH_SIZE; + Node* n = Node::make(Hash(bytes, position)); leaf_nodes.push_back(n); } diff --git a/test/unit_tests.cpp b/test/unit_tests.cpp index ce7ce8d..dcd95fb 100644 --- a/test/unit_tests.cpp +++ b/test/unit_tests.cpp @@ -277,6 +277,26 @@ TEST_CASE("Empty tree") REQUIRE_NOTHROW(merkle::Tree dt(buffer)); // NOLINT(misc-const-correctness) } +TEST_CASE("TreeT rejects invalid serialised leaf data") +{ + std::vector excessive_count; + merkle::serialise_uint64_t( + std::numeric_limits::max(), excessive_count); + merkle::serialise_uint64_t(0, excessive_count); + REQUIRE_THROWS_AS( + (void)merkle::Tree(excessive_count), std::runtime_error); + + std::vector truncated_hashes; + merkle::serialise_uint64_t(2, truncated_hashes); + merkle::serialise_uint64_t(0, truncated_hashes); + truncated_hashes.resize( + truncated_hashes.size() + merkle::Hash::size_bytes); + REQUIRE_THROWS_WITH_AS( + (void)merkle::Tree(truncated_hashes), + "not enough bytes", + std::runtime_error); +} + TEST_CASE("One-node tree") { merkle::Tree::Hash h; From a8634fd7a1f8dc03750a16425e4db7ea45ade938 Mon Sep 17 00:00:00 2001 From: achamayou Date: Mon, 24 Aug 2026 19:47:39 +0100 Subject: [PATCH 2/4] Reject unrepresentable deserialised trees Validate the combined leaf count against Node::size, use width-safe full-tree sizing, and preflight hashes for flushed subtrees. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d55d93ce-932b-421d-9dd7-0d037b06a676 --- merklecpp.h | 36 ++++++++++++++++++++++++++++------ test/unit_tests.cpp | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/merklecpp.h b/merklecpp.h index 3ef1705..12eba68 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -734,13 +734,19 @@ namespace merkle { return false; } - const size_t max_size = height == size_digits ? - std::numeric_limits::max() : - (size_t{1} << height) - 1; + const size_t max_size = full_size(height); assert(size <= max_size); return size == max_size; } + static size_t full_size(uint8_t height) + { + constexpr size_t size_digits = std::numeric_limits::digits; + assert(height <= size_digits); + return height == size_digits ? std::numeric_limits::max() : + (size_t{1} << height) - 1; + } + /// @brief Updates the tree size and height of the subtree under a node void update_sizes() { @@ -1548,14 +1554,32 @@ namespace merkle clear(); const size_t num_leaf_nodes = deserialise_size_t(bytes, position); - num_flushed = deserialise_size_t(bytes, position); + const size_t deserialised_num_flushed = + deserialise_size_t(bytes, position); + + // A binary tree has 2 * leaves - 1 nodes, which must fit in Node::size. + constexpr size_t max_num_leaves = + std::numeric_limits::max() / 2 + 1; + if ( + deserialised_num_flushed > max_num_leaves || + num_leaf_nodes > max_num_leaves - deserialised_num_flushed) + { + throw std::runtime_error("serialised tree exceeds platform limits"); + } + + size_t num_hashes = num_leaf_nodes; + for (size_t it = deserialised_num_flushed; it != 0; it >>= 1) + { + num_hashes += it & 0x01; + } if ( position > bytes.size() || - num_leaf_nodes > (bytes.size() - position) / HASH_SIZE) + num_hashes > (bytes.size() - position) / HASH_SIZE) { throw std::runtime_error("not enough bytes"); } + num_flushed = deserialised_num_flushed; leaf_nodes.reserve(num_leaf_nodes); for (size_t i = 0; i < num_leaf_nodes; i++) { @@ -1576,7 +1600,7 @@ namespace merkle MERKLECPP_TRACE(MERKLECPP_TOUT << "+";); auto n = Node::make(h); n->height = level_no + 1; - n->size = (1 << n->height) - 1; + n->size = Node::full_size(n->height); assert(n->invariant()); level.insert(level.begin(), n); } diff --git a/test/unit_tests.cpp b/test/unit_tests.cpp index dcd95fb..4d55735 100644 --- a/test/unit_tests.cpp +++ b/test/unit_tests.cpp @@ -295,6 +295,53 @@ TEST_CASE("TreeT rejects invalid serialised leaf data") (void)merkle::Tree(truncated_hashes), "not enough bytes", std::runtime_error); + + std::vector overflowing_leaf_count; + merkle::serialise_uint64_t(1, overflowing_leaf_count); + merkle::serialise_uint64_t( + std::numeric_limits::max(), overflowing_leaf_count); + overflowing_leaf_count.resize( + overflowing_leaf_count.size() + merkle::Hash::size_bytes); + REQUIRE_THROWS_WITH_AS( + (void)merkle::Tree(overflowing_leaf_count), + "serialised tree exceeds platform limits", + std::runtime_error); + + std::vector unrepresentable_tree_size; + merkle::serialise_uint64_t(1, unrepresentable_tree_size); + merkle::serialise_uint64_t( + std::numeric_limits::max() / 2 + 1, unrepresentable_tree_size); + REQUIRE_THROWS_WITH_AS( + (void)merkle::Tree(unrepresentable_tree_size), + "serialised tree exceeds platform limits", + std::runtime_error); + + std::vector truncated_extra_hash; + merkle::serialise_uint64_t(1, truncated_extra_hash); + merkle::serialise_uint64_t(1, truncated_extra_hash); + truncated_extra_hash.resize( + truncated_extra_hash.size() + merkle::Hash::size_bytes); + REQUIRE_THROWS_WITH_AS( + (void)merkle::Tree(truncated_extra_hash), + "not enough bytes", + std::runtime_error); +} + +TEST_CASE("TreeT deserialises flushed counts beyond signed shift width") +{ + constexpr auto flushed_bit = std::numeric_limits::digits; + if constexpr (flushed_bit + 1 < std::numeric_limits::digits) + { + const size_t num_flushed = size_t{1} << flushed_bit; + std::vector bytes; + merkle::serialise_uint64_t(1, bytes); + merkle::serialise_uint64_t(num_flushed, bytes); + bytes.resize(bytes.size() + 2 * merkle::Hash::size_bytes); + + merkle::Tree tree(bytes); + REQUIRE(tree.num_leaves() == num_flushed + 1); + REQUIRE(tree.size() == 2 * num_flushed + 1); + } } TEST_CASE("One-node tree") From ecfd076eb2ba334f7b858f227dbf2b9f2e21cc29 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 25 Aug 2026 13:37:50 +0100 Subject: [PATCH 3/4] Complete tree deserialisation validation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- merklecpp.h | 14 ++++++++++++-- test/unit_tests.cpp | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/merklecpp.h b/merklecpp.h index 12eba68..de1f7f7 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -97,9 +97,14 @@ namespace merkle const std::vector& bytes, size_t& index) { const auto value = deserialise_uint64_t(bytes, index); - if (value > std::numeric_limits::max()) + if constexpr ( + std::numeric_limits::digits < + std::numeric_limits::digits) { - throw std::runtime_error("serialised value exceeds platform limits"); + if (value > std::numeric_limits::max()) + { + throw std::runtime_error("serialised value exceeds platform limits"); + } } return static_cast(value); } @@ -1557,6 +1562,11 @@ namespace merkle const size_t deserialised_num_flushed = deserialise_size_t(bytes, position); + if (num_leaf_nodes == 0 && deserialised_num_flushed != 0) + { + throw std::runtime_error("serialised tree has no retained leaves"); + } + // A binary tree has 2 * leaves - 1 nodes, which must fit in Node::size. constexpr size_t max_num_leaves = std::numeric_limits::max() / 2 + 1; diff --git a/test/unit_tests.cpp b/test/unit_tests.cpp index 4d55735..975f376 100644 --- a/test/unit_tests.cpp +++ b/test/unit_tests.cpp @@ -279,6 +279,14 @@ TEST_CASE("Empty tree") TEST_CASE("TreeT rejects invalid serialised leaf data") { + for (size_t size = 0; size < 2 * sizeof(uint64_t); size++) + { + CAPTURE(size); + const std::vector truncated_header(size, 0); + REQUIRE_THROWS_AS( + (void)merkle::Tree(truncated_header), std::out_of_range); + } + std::vector excessive_count; merkle::serialise_uint64_t( std::numeric_limits::max(), excessive_count); @@ -296,6 +304,16 @@ TEST_CASE("TreeT rejects invalid serialised leaf data") "not enough bytes", std::runtime_error); + std::vector no_retained_leaves; + merkle::serialise_uint64_t(0, no_retained_leaves); + merkle::serialise_uint64_t(1, no_retained_leaves); + no_retained_leaves.resize( + no_retained_leaves.size() + merkle::Hash::size_bytes); + REQUIRE_THROWS_WITH_AS( + (void)merkle::Tree(no_retained_leaves), + "serialised tree has no retained leaves", + std::runtime_error); + std::vector overflowing_leaf_count; merkle::serialise_uint64_t(1, overflowing_leaf_count); merkle::serialise_uint64_t( From 05d015eec6dbf3ada5a69a62e645e96fb5d0ae97 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 25 Aug 2026 13:51:22 +0100 Subject: [PATCH 4/4] Suppress intentional width warning Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- merklecpp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merklecpp.h b/merklecpp.h index de1f7f7..ca02e9c 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -98,7 +98,7 @@ namespace merkle { const auto value = deserialise_uint64_t(bytes, index); if constexpr ( - std::numeric_limits::digits < + std::numeric_limits::digits < // NOLINT(misc-redundant-expression) std::numeric_limits::digits) { if (value > std::numeric_limits::max())