What the developer wrote
Authored by Lee Clagett
Improve unpack reserve handling
What changed, and why it matters
This commit hardens how Monero's serialization code reserves memory when unpacking data from untrusted network or storage inputs. Before, a maliciously crafted message could claim an enormous number of elements and trick the software into reserving a huge chunk of RAM, enabling memory exhaustion (denial of service). The patch now caps the reserved memory based on how many bytes are actually left in the input buffer and the largest plausible compression ratio for the element type, so a small blob cannot trigger a giant allocation.
Treat this as a security hardening fix and include it in release notes. Users and integrators should upgrade to a release containing this commit. Operators running nodes that accept untrusted P2P traffic should prioritize the patch because the deserialization path is reachable from the network.
Security signals we found
Memory-reserve cap based on actual remaining input bytes
Defense against oversized container count fields during deserialization
Addition of unit test asserting bounded capacity after failed deserialization
Use of numeric_limits overflow guard before multiplication
Evidence from the diff
The change is in src/serialization/container.h. do_reserve() now takes the remaining archive byte count (ar.remaining_bytes()) and computes an upper bound on how many elements could realistically be encoded in that space. It divides remaining bytes by sizeof(value_type), clamps to at least 1, multiplies by a per-type max_compression_ratio (1 for blobs, sizeof(T) for varints, 1 for char/unsigned char, 4 otherwise), and then reserves only min(claimed_count, computed_bound). The unit test demonstrates the defense: a blob containing a varint count of 100 but only 100 zero bytes is parsed into a vector
Changed components
src/serialization/container.htests/unit_tests/serialization.cppInspect captured patch +39 / −2
diff --git a/src/serialization/container.h b/src/serialization/container.h
index 508b509..5d2ac00 100644
--- a/src/serialization/container.h
+++ b/src/serialization/container.h
@@ -30,11 +30,15 @@
#pragma once
+#include <algorithm>
#include <cstdint>
#include <cstddef>
+#include <limits>
#include <string>
#include <type_traits>
+#include "serialization/serialization.h"
+
namespace serialization
{
namespace detail
@@ -76,7 +80,26 @@ namespace serialization
template<typename... C>
void do_reserve(const C&...) {}
template<typename C>
- auto do_reserve(C &c, std::size_t N) -> decltype(c.reserve(N)) { return c.reserve(N); }
+ auto do_reserve(C &c, std::size_t N, std::size_t B) -> decltype(c.reserve(N))
+ {
+ using T = typename C::value_type;
+
+ static constexpr std::size_t max_compression_ratio =
+ is_blob_type<T>::type::value ? 1 :
+ use_container_varint<T>() ? sizeof(T) :
+ (std::is_same<T, char>::value || std::is_same<T, unsigned char>::value) ? 1:
+ 4; // default
+
+ // max compression ratio for upfront memory usage
+ B /= sizeof(T);
+ B = std::max(std::size_t(1), B);
+ if (std::numeric_limits<std::size_t>::max() / max_compression_ratio <= B)
+ B = std::numeric_limits<std::size_t>::max();
+ else
+ B *= max_compression_ratio;
+
+ return c.reserve(std::min(N, B));
+ }
// The value_type of STL map-like containers come in the form std::pair<const K, V>.
// Since we can't {de}serialize const types in this lib, we must convert this to std::pair<K, V>
@@ -104,7 +127,7 @@ bool do_serialize_container(Archive<false> &ar, C &v)
return false;
}
- ::serialization::detail::do_reserve(v, cnt);
+ ::serialization::detail::do_reserve(v, cnt, ar.remaining_bytes());
for (size_t i = 0; i < cnt; i++) {
if (i > 0)
diff --git a/tests/unit_tests/serialization.cpp b/tests/unit_tests/serialization.cpp
index 6cf9be8..893d493 100644
--- a/tests/unit_tests/serialization.cpp
+++ b/tests/unit_tests/serialization.cpp
@@ -372,6 +372,20 @@ TEST(Serialization, serializes_vector_int64_as_fixed_int)
ASSERT_EQ(57, blob.size());
}
+TEST(Serialization, deserializes_vector_reserve)
+{
+ std::vector<int64_t> v;
+ string blob;
+
+ tools::write_varint(std::back_inserter(blob), unsigned(100));
+ blob.append(std::string(100, 0));
+
+ ASSERT_LT(v.capacity(), 20);
+ ASSERT_FALSE(serialization::parse_binary(blob, v));
+ ASSERT_LT(v.capacity(), 100); // could fail if lib allocates more in reserve call
+}
+
+
namespace
{
template<typename T>
Why this scored 58/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.