Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 71 additions & 15 deletions src/serialize.h
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ static inline Wrapper<Formatter, T&> Using(T&& t) { return Wrapper<Formatter, T&
#define VARINT(obj) Using<VarIntFormatter<VarIntMode::DEFAULT>>(obj)
#define COMPACTSIZE(obj) Using<CompactSizeFormatter<true>>(obj)
#define LIMITED_STRING(obj,n) Using<LimitedStringFormatter<n>>(obj)
#define LIMITED_VECTOR(obj,n) Using<LimitedVectorFormatter<n>>(obj)

/** TODO: describe DynamicBitSet */
struct DynamicBitSetFormatter
Expand Down Expand Up @@ -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<class Formatter, typename Stream, typename V>
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:
Expand Down Expand Up @@ -796,25 +823,12 @@ struct VectorFormatter
template<typename Stream, typename V>
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<Formatter>(s, v, ReadCompactSize(s));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Refactor reorders Formatter construction relative to clear/count-read

The pre-refactor VectorFormatter::Unser default-constructed Formatter formatter; before calling v.clear() and ReadCompactSize(s). The shared helper now constructs the Formatter inside detail::UnserializeVectorContents, after the vector is cleared and the count has been consumed from the stream. For a Formatter whose default constructor has side effects or can throw, a construction failure would now leave the stream advanced past the count and the destination vector cleared — an observable difference in a change advertised as behavior-preserving. This is inert for every formatter in the tree today (all are stateless empty structs with trivial default constructors), so it is only worth noting for the refactor's stated invariant, not a defect. If you want to preserve the exact original ordering, construct the Formatter in VectorFormatter::Unser and pass it into the shared helper by reference.

source: ['codex']

};
};


/**
* Forward declarations
*/
Expand Down Expand Up @@ -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<class Formatter = DefaultFormatter, typename Stream, typename V>
[[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<Formatter>(s, v, size);
return true;
}

/** Compile-time bounded vector formatter, usable inside READWRITE via
* `Using<LimitedVectorFormatter<Limit>>(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<size_t Limit, class ElemFormatter = DefaultFormatter>
struct LimitedVectorFormatter
{
template<typename Stream, typename V>
void Ser(Stream& s, const V& v)
{
VectorFormatter<ElemFormatter>{}.Ser(s, v);
}

template<typename Stream, typename V>
void Unser(Stream& s, V& v)
{
if (!UnserializeVectorWithMaxSize<ElemFormatter>(s, v, Limit)) {
throw std::ios_base::failure("Vector length limit exceeded");
}
}
};

/**
* string
*/
Expand Down
202 changes: 202 additions & 0 deletions src/test/serialize_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@

#include <stdint.h>

#include <limits>
#include <stdexcept>
#include <string_view>
#include <vector>

#include <boost/test/unit_test.hpp>

BOOST_FIXTURE_TEST_SUITE(serialize_tests, BasicTestingSetup)
Expand Down Expand Up @@ -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<typename Stream, typename T>
void Ser(Stream& s, const T& t) { Serialize(s, t); }

template<typename Stream, typename T>
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<int> 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<int> v{1, 2, 3};
ss << v;

DataStream ref;
ref << v;
BOOST_CHECK_EQUAL_COLLECTIONS(ss.begin(), ss.end(), ref.begin(), ref.end());

std::vector<int> 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<int>{};
std::vector<int> out{99};
BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/0));
BOOST_CHECK(out.empty());
}
{
DataStream ss;
ss << std::vector<int>{1};
std::vector<int> 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<int> v{10, 20, 30, 40};
ss << v;
std::vector<int> out;
BOOST_CHECK(UnserializeVectorWithMaxSize(ss, out, /*max_size=*/4));
BOOST_CHECK(out == v);
}
{
DataStream ss;
const std::vector<int> v{10, 20, 30, 40, 50};
ss << v;
std::vector<int> out;
CountingFormatter::Reset();
BOOST_CHECK(!UnserializeVectorWithMaxSize<CountingFormatter>(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<int>{7, 8, 9};
std::vector<int> out;
CountingFormatter::Reset();
BOOST_CHECK(UnserializeVectorWithMaxSize<CountingFormatter>(ss, out, /*max_size=*/8));
BOOST_CHECK_EQUAL(CountingFormatter::unser_calls, 3U);
BOOST_CHECK((out == std::vector<int>{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<uint32_t>::max(),
"MAX_SIZE must fit in a 32-bit CompactSize prefix for this test");
DataStream ss;
ss << uint8_t{0xfe};
ss << static_cast<uint32_t>(MAX_SIZE);
std::vector<int> out;
CountingFormatter::Reset();
BOOST_CHECK(!UnserializeVectorWithMaxSize<CountingFormatter>(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<uint32_t>(MAX_SIZE + 1);
std::vector<int> out;
BOOST_CHECK_THROW((void)UnserializeVectorWithMaxSize<CountingFormatter>(ss, out, /*max_size=*/8),
std::ios_base::failure);
}
{
DataStream ss;
ss << uint8_t{0xff};
ss << uint64_t{0x100000000ULL + MAX_SIZE};
std::vector<int> out;
BOOST_CHECK_THROW((void)UnserializeVectorWithMaxSize<CountingFormatter>(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<int> 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<int>{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<int> big{1, 2, 3, 4, 5, 6, 7};
ss << Using<LimitedVectorFormatter<5>>(big);

DataStream ref;
ref << big;
BOOST_CHECK_EQUAL_COLLECTIONS(ss.begin(), ss.end(), ref.begin(), ref.end());

std::vector<int> out;
BOOST_REQUIRE_NO_THROW(ss >> out);
BOOST_CHECK(out == big);
}
}

BOOST_AUTO_TEST_SUITE_END()