From af6a0fda99ab9108f2d1439d85c8bfe5954ad55f Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Thu, 9 Jul 2026 21:00:15 -0500 Subject: [PATCH] feat(serialize): add bounded-vector deserialization primitives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a runtime and a compile-time bounded reader for CompactSize-prefixed vectors so downstream call sites can reject grossly oversized wire counts before any element decode or allocation. Both primitives preserve the ordinary std::vector wire format — the limit is a deserialization safety property. - Factor the batched vector element-decode loop out of VectorFormatter::Unser into detail::UnserializeVectorContents, and reuse it from both new primitives so the DoS-resistant 5 MiB batching is not duplicated. - UnserializeVectorWithMaxSize(stream, vec, max_size) reads the CompactSize prefix and rejects a count above the caller's max_size before any element decode, returning false without consuming an element. A count above the generic MAX_SIZE cap throws from ReadCompactSize as malformed; since a realistic max_size is far below MAX_SIZE (you reach MAX_PROTOCOL_MESSAGE_LENGTH long before MAX_SIZE), the caller's own limit is the gate that fires in practice. The helper may otherwise throw on an unrelated serialization failure. - LimitedVectorFormatter is the READWRITE-friendly wrapper; a LIMITED_VECTOR(obj, n) macro mirrors the existing LIMITED_STRING convention. Ser delegates to VectorFormatter so the write side stays unbounded and interoperable with the standard reader. Cover the primitives with focused unit tests: within/exact/over/zero-limit boundaries, wire-format identity vs the ordinary vector encoding, that Ser remains unbounded, and — via a CountingFormatter that tallies Unser calls — that a MAX_SIZE wire count is rejected before any element decode occurs, while counts above MAX_SIZE throw at the CompactSize level. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/serialize.h | 86 ++++++++++++--- src/test/serialize_tests.cpp | 202 +++++++++++++++++++++++++++++++++++ 2 files changed, 273 insertions(+), 15 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index d7c4f7f3f3e6..73cec742b81d 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -591,6 +591,7 @@ static inline Wrapper Using(T&& t) { return Wrapper>(obj) #define COMPACTSIZE(obj) Using>(obj) #define LIMITED_STRING(obj,n) Using>(obj) +#define LIMITED_VECTOR(obj,n) Using>(obj) /** TODO: describe DynamicBitSet */ struct DynamicBitSetFormatter @@ -767,6 +768,32 @@ struct LimitedStringFormatter } }; +namespace detail { +/** Shared vector element-decode loop used by VectorFormatter and by the + * runtime-bounded reader. `size` must already have been validated by the + * caller against any semantic limit — this helper only performs the + * batched, size-capped allocation the vector unserialize path relies on for + * DoS resistance. */ +template +void UnserializeVectorContents(Stream& s, V& v, size_t size) +{ + Formatter formatter; + size_t allocated = 0; + while (allocated < size) { + // For DoS prevention, do not blindly allocate as much as the stream claims to contain. + // Instead, allocate in 5MiB batches, so that an attacker actually needs to provide + // X MiB of data to make us allocate X+5 Mib. + static_assert(sizeof(typename V::value_type) <= MAX_VECTOR_ALLOCATE, "Vector element size too large"); + allocated = std::min(size, allocated + MAX_VECTOR_ALLOCATE / sizeof(typename V::value_type)); + v.reserve(allocated); + while (v.size() < allocated) { + v.emplace_back(); + formatter.Unser(s, v.back()); + } + } +} +} // namespace detail + /** Formatter to serialize/deserialize vector elements using another formatter * * Example: @@ -796,25 +823,12 @@ struct VectorFormatter template void Unser(Stream& s, V& v) { - Formatter formatter; v.clear(); - size_t size = ReadCompactSize(s); - size_t allocated = 0; - while (allocated < size) { - // For DoS prevention, do not blindly allocate as much as the stream claims to contain. - // Instead, allocate in 5MiB batches, so that an attacker actually needs to provide - // X MiB of data to make us allocate X+5 Mib. - static_assert(sizeof(typename V::value_type) <= MAX_VECTOR_ALLOCATE, "Vector element size too large"); - allocated = std::min(size, allocated + MAX_VECTOR_ALLOCATE / sizeof(typename V::value_type)); - v.reserve(allocated); - while (v.size() < allocated) { - v.emplace_back(); - formatter.Unser(s, v.back()); - } - } + detail::UnserializeVectorContents(s, v, ReadCompactSize(s)); }; }; + /** * Forward declarations */ @@ -955,6 +969,48 @@ struct DefaultFormatter static void Unser(Stream& s, T& t) { Unserialize(s, t); } }; +/** Read a CompactSize-prefixed vector while rejecting counts above + * `max_size` before any element decode or allocation occurs. + * + * Returns false (with `v` cleared, stream positioned just past the count) if + * the encoded count exceeds `max_size`. May throw exception if encountering + * unrelated serialization failure. */ +template +[[nodiscard]] bool UnserializeVectorWithMaxSize(Stream& s, V& v, size_t max_size) +{ + v.clear(); + const size_t size = ReadCompactSize(s); + if (size > max_size) { + return false; + } + detail::UnserializeVectorContents(s, v, size); + return true; +} + +/** Compile-time bounded vector formatter, usable inside READWRITE via + * `Using>(v)` or the LIMITED_VECTOR(v, Limit) + * macro. Emits the ordinary vector wire format — the Limit is a + * deserialization safety property only, so producers stay compatible with + * the unbounded reader on the other side. On deserialization, a wire count + * above Limit throws before any element is decoded or memory allocated. */ +template +struct LimitedVectorFormatter +{ + template + void Ser(Stream& s, const V& v) + { + VectorFormatter{}.Ser(s, v); + } + + template + void Unser(Stream& s, V& v) + { + if (!UnserializeVectorWithMaxSize(s, v, Limit)) { + throw std::ios_base::failure("Vector length limit exceeded"); + } + } +}; + /** * string */ diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index d0b6cc774ccf..a51b98a1ba80 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -10,6 +10,11 @@ #include +#include +#include +#include +#include + #include BOOST_FIXTURE_TEST_SUITE(serialize_tests, BasicTestingSetup) @@ -306,4 +311,201 @@ BOOST_AUTO_TEST_CASE(class_methods) } } +namespace { +// Element formatter that counts Unser invocations, so a test can prove the +// bounded reader rejected an oversized wire count *before* touching any +// element. Ser delegates to the default Serialize so wire compatibility is +// preserved. +struct CountingFormatter +{ + static inline size_t unser_calls = 0; + static void Reset() { unser_calls = 0; } + + template + void Ser(Stream& s, const T& t) { Serialize(s, t); } + + template + void Unser(Stream& s, T& t) + { + ++unser_calls; + Unserialize(s, t); + } +}; + +bool IsLimitExceededFailure(const std::ios_base::failure& e) +{ + return std::string_view{e.what()}.find("Vector length limit exceeded") != std::string_view::npos; +} + +// Struct with a compile-time bounded vector member, so the compile-time +// formatter can be exercised end-to-end via ordinary << / >> and READWRITE. +struct LimitedVecFive +{ + std::vector v; + SERIALIZE_METHODS(LimitedVecFive, obj) { READWRITE(LIMITED_VECTOR(obj.v, 5)); } +}; +} // namespace + +BOOST_AUTO_TEST_CASE(unserialize_vector_with_max_size) +{ + // Within-limit round trip: an ordinary vector encoding decodes cleanly and + // the wire bytes are byte-identical to the unbounded writer's output, so + // this helper stays interchangeable with the standard vector Unserialize. + { + DataStream ss; + const std::vector v{1, 2, 3}; + ss << v; + + DataStream ref; + ref << v; + BOOST_CHECK_EQUAL_COLLECTIONS(ss.begin(), ss.end(), ref.begin(), ref.end()); + + std::vector out; + BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/8)); + BOOST_CHECK(out == v); + BOOST_CHECK_EQUAL(ss.size(), 0U); + } + + // Zero limit rejects any non-empty vector and accepts an empty one. + { + DataStream ss; + ss << std::vector{}; + std::vector out{99}; + BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/0)); + BOOST_CHECK(out.empty()); + } + { + DataStream ss; + ss << std::vector{1}; + std::vector out; + BOOST_CHECK(!UnserializeVectorWithMaxSize(ss, out, /*max_size=*/0)); + BOOST_CHECK(out.empty()); + } + + // Exact-limit boundary decodes; one-over boundary is rejected and no + // element decoding happens. + { + DataStream ss; + const std::vector v{10, 20, 30, 40}; + ss << v; + std::vector out; + BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/4)); + BOOST_CHECK(out == v); + } + { + DataStream ss; + const std::vector v{10, 20, 30, 40, 50}; + ss << v; + std::vector out; + CountingFormatter::Reset(); + BOOST_CHECK(!UnserializeVectorWithMaxSize(ss, out, /*max_size=*/4)); + BOOST_CHECK(out.empty()); + BOOST_CHECK_EQUAL(CountingFormatter::unser_calls, 0U); + } + + // Custom element formatter path: within-limit, verify the element formatter + // actually runs; over-limit, verify it does not. + { + DataStream ss; + ss << std::vector{7, 8, 9}; + std::vector out; + CountingFormatter::Reset(); + BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/8)); + BOOST_CHECK_EQUAL(CountingFormatter::unser_calls, 3U); + BOOST_CHECK((out == std::vector{7, 8, 9})); + } + + // A wire count at MAX_SIZE still hits the caller's own limit (8) first and + // returns false without decoding any element. + { + static_assert(MAX_SIZE < std::numeric_limits::max(), + "MAX_SIZE must fit in a 32-bit CompactSize prefix for this test"); + DataStream ss; + ss << uint8_t{0xfe}; + ss << static_cast(MAX_SIZE); + std::vector out; + CountingFormatter::Reset(); + BOOST_CHECK(!UnserializeVectorWithMaxSize(ss, out, /*max_size=*/8)); + BOOST_CHECK(out.empty()); + BOOST_CHECK_EQUAL(CountingFormatter::unser_calls, 0U); + } + + // Counts above MAX_SIZE throw from ReadCompactSize before the caller's + // gate is consulted — these are malformed at the CompactSize level. + { + DataStream ss; + ss << uint8_t{0xfe}; + ss << static_cast(MAX_SIZE + 1); + std::vector out; + BOOST_CHECK_THROW((void)UnserializeVectorWithMaxSize(ss, out, /*max_size=*/8), + std::ios_base::failure); + } + { + DataStream ss; + ss << uint8_t{0xff}; + ss << uint64_t{0x100000000ULL + MAX_SIZE}; + std::vector out; + BOOST_CHECK_THROW((void)UnserializeVectorWithMaxSize(ss, out, /*max_size=*/8), + std::ios_base::failure); + } +} + +BOOST_AUTO_TEST_CASE(limited_vector_formatter_compile_time) +{ + // The compile-time formatter's wire format matches the ordinary vector + // encoding — a struct wrapping std::vector under LIMITED_VECTOR must + // produce the same bytes as the raw vector. + { + DataStream ss_lim; + DataStream ss_ord; + LimitedVecFive obj{{1, 2, 3}}; + ss_lim << obj; + ss_ord << obj.v; + BOOST_CHECK_EQUAL_COLLECTIONS(ss_lim.begin(), ss_lim.end(), + ss_ord.begin(), ss_ord.end()); + + LimitedVecFive round; + ss_lim >> round; + BOOST_CHECK(round.v == obj.v); + } + + // At the exact limit: round-trips. + { + DataStream ss; + LimitedVecFive obj{{1, 2, 3, 4, 5}}; + ss << obj; + LimitedVecFive round; + BOOST_REQUIRE_NO_THROW(ss >> round); + BOOST_CHECK(round.v == obj.v); + } + + // Over the limit: rejected with the sentinel failure, before element decode. + { + DataStream ss; + ss << std::vector{1, 2, 3, 4, 5, 6}; + LimitedVecFive round; + BOOST_CHECK_EXCEPTION(ss >> round, std::ios_base::failure, IsLimitExceededFailure); + BOOST_CHECK(round.v.empty()); + } + + // Serialization stays unbounded: writing an over-limit vector through the + // formatter must still emit the ordinary wire format, so producers remain + // compatible with the standard reader. Reading it back through the + // unbounded std::vector path succeeds even though the LIMITED_VECTOR reader + // would reject it. + { + DataStream ss; + const std::vector big{1, 2, 3, 4, 5, 6, 7}; + ss << Using>(big); + + DataStream ref; + ref << big; + BOOST_CHECK_EQUAL_COLLECTIONS(ss.begin(), ss.end(), ref.begin(), ref.end()); + + std::vector out; + BOOST_REQUIRE_NO_THROW(ss >> out); + BOOST_CHECK(out == big); + } +} + BOOST_AUTO_TEST_SUITE_END()