From 91e7d22173c8662639dd739695676c2fe879e9aa Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Fri, 30 Oct 2026 13:51:04 -0700 Subject: [PATCH 1/5] KLL: check k, the number of levels and the levels array when deserializing deserialize() used the serialized levels array without checking it. A level beyond the capacity, or a level that starts before the previous one, put level boundaries outside the items buffer, and the bytes overload read the levels array without checking the input size. Both overloads now check that k >= MIN_K, that there is at least one level, that the input holds the levels array, and that each level starts at or after the previous one and no later than the capacity. Co-Authored-By: Claude Opus 5.5 --- kll/include/kll_sketch.hpp | 3 ++ kll/include/kll_sketch_impl.hpp | 35 +++++++++++++++++ kll/test/kll_sketch_test.cpp | 70 +++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index d672c419..39cac445 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -591,10 +591,13 @@ class kll_sketch { uint32_t safe_level_size(uint8_t level) const; uint32_t get_num_retained_above_level_zero() const; + static void check_k(uint16_t k); static void check_m(uint8_t m); static void check_preamble_ints(uint8_t preamble_ints, uint8_t flags_byte); static void check_serial_version(uint8_t serial_version); static void check_family_id(uint8_t family_id); + static void check_num_levels(uint8_t num_levels); + static void check_levels(const vector_u32& levels); void check_sorting() const; diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index b12a39c8..a1d0c4da 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -450,6 +450,7 @@ kll_sketch kll_sketch::deserialize(std::istream& is, const Ser const auto m = read(is); read(is); // skip unused byte + check_k(k); check_m(m); check_preamble_ints(preamble_ints, flags_byte); check_serial_version(serial_version); @@ -472,6 +473,7 @@ kll_sketch kll_sketch::deserialize(std::istream& is, const Ser min_k = read(is); num_levels = read(is); read(is); // skip unused byte + check_num_levels(num_levels); } vector_u32 levels(num_levels + 1, 0, allocator); const uint32_t capacity(kll_helper::compute_total_capacity(k, m, num_levels)); @@ -480,8 +482,10 @@ kll_sketch kll_sketch::deserialize(std::istream& is, const Ser } else { // the last integer in levels_ is not serialized because it can be derived read(is, levels.data(), sizeof(levels[0]) * num_levels); + if (!is.good()) throw std::runtime_error("error reading from std::istream"); } levels[num_levels] = capacity; + check_levels(levels); optional min_item; optional max_item; if (!is_single_item) { @@ -537,6 +541,7 @@ kll_sketch kll_sketch::deserialize(const void* bytes, size_t s ptr += copy_from_mem(ptr, m); ptr += sizeof(uint8_t); // skip unused byte + check_k(k); check_m(m); check_preamble_ints(preamble_ints, flags_byte); check_serial_version(serial_version); @@ -560,6 +565,7 @@ kll_sketch kll_sketch::deserialize(const void* bytes, size_t s ptr += copy_from_mem(ptr, min_k); ptr += copy_from_mem(ptr, num_levels); ptr += sizeof(uint8_t); // skip unused byte + check_num_levels(num_levels); } vector_u32 levels(num_levels + 1, 0, allocator); const uint32_t capacity(kll_helper::compute_total_capacity(k, m, num_levels)); @@ -567,9 +573,11 @@ kll_sketch kll_sketch::deserialize(const void* bytes, size_t s levels[0] = capacity - 1; } else { // the last integer in levels_ is not serialized because it can be derived + ensure_minimum_memory(size, (ptr - static_cast(bytes)) + sizeof(levels[0]) * num_levels); ptr += copy_from_mem(ptr, levels.data(), sizeof(levels[0]) * num_levels); } levels[num_levels] = capacity; + check_levels(levels); optional min_item; optional max_item; if (!is_single_item) { @@ -867,6 +875,14 @@ uint32_t kll_sketch::get_num_retained_above_level_zero() const { return levels_[num_levels_] - levels_[1]; } +template +void kll_sketch::check_k(uint16_t k) { + if (k < kll_constants::MIN_K) { + throw std::invalid_argument("Possible corruption: K must be >= " + std::to_string(kll_constants::MIN_K) + + ": " + std::to_string(k)); + } +} + template void kll_sketch::check_m(uint8_t m) { if (m != kll_constants::DEFAULT_M) { @@ -909,6 +925,25 @@ void kll_sketch::check_family_id(uint8_t family_id) { } } +template +void kll_sketch::check_num_levels(uint8_t num_levels) { + if (num_levels == 0) { + throw std::invalid_argument("Possible corruption: number of levels must be > 0"); + } +} + +// The last entry is the capacity. Each level must start at or after the previous one, +// so every level lies within the items buffer. +template +void kll_sketch::check_levels(const vector_u32& levels) { + for (size_t i = 1; i < levels.size(); ++i) { + if (levels[i - 1] > levels[i]) { + throw std::invalid_argument("Possible corruption: levels[" + std::to_string(i - 1) + "] = " + + std::to_string(levels[i - 1]) + " > levels[" + std::to_string(i) + "] = " + std::to_string(levels[i])); + } + } +} + template string kll_sketch::to_string(bool print_levels, bool print_items) const { // Using a temporary stream for implementation here does not comply with AllocatorAwareContainer requirements. diff --git a/kll/test/kll_sketch_test.cpp b/kll/test/kll_sketch_test.cpp index b8d0b031..cb09473e 100644 --- a/kll/test/kll_sketch_test.cpp +++ b/kll/test/kll_sketch_test.cpp @@ -835,4 +835,74 @@ TEST_CASE("kll sketch", "[kll_sketch]") { REQUIRE(test_allocator_total_bytes == 0); } + +TEST_CASE("kll sketch: deserialize rejects inconsistent levels", "[kll_sketch]") { + using vector_bytes = kll_sketch::vector_bytes; + const size_t k_offset = 4; + const size_t num_levels_offset = 18; + const size_t levels_offset = 20; + auto put_u32 = [](vector_bytes& bytes, size_t offset, uint32_t value) { + std::memcpy(bytes.data() + offset, &value, sizeof(value)); + }; + auto get_u32 = [](const vector_bytes& bytes, size_t offset) { + uint32_t value; + std::memcpy(&value, bytes.data() + offset, sizeof(value)); + return value; + }; + auto check_rejected = [](const vector_bytes& bytes) { + REQUIRE_THROWS_AS(kll_sketch::deserialize(bytes.data(), bytes.size()), std::invalid_argument); + std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); + s.write(reinterpret_cast(bytes.data()), bytes.size()); + REQUIRE_THROWS_AS(kll_sketch::deserialize(s), std::invalid_argument); + }; + + kll_sketch one_level(8); + for (int i = 0; i < 3; ++i) one_level.update(i); + kll_sketch three_levels(8); + for (int i = 0; i < 40; ++i) three_levels.update(i); + REQUIRE(three_levels.serialize()[num_levels_offset] == 3); + + SECTION("first level beyond capacity") { + auto bytes = one_level.serialize(); + put_u32(bytes, levels_offset, 1009); + check_rejected(bytes); + } + + SECTION("level beyond capacity") { + auto bytes = three_levels.serialize(); + put_u32(bytes, levels_offset + sizeof(uint32_t), 100000); + check_rejected(bytes); + } + + SECTION("decreasing levels") { + auto bytes = three_levels.serialize(); + REQUIRE(get_u32(bytes, levels_offset) > 0); + put_u32(bytes, levels_offset + sizeof(uint32_t), 0); + check_rejected(bytes); + } + + SECTION("zero levels") { + auto bytes = three_levels.serialize(); + bytes[num_levels_offset] = 0; + check_rejected(bytes); + } + + SECTION("k below minimum") { + auto bytes = three_levels.serialize(); + const uint16_t k = 4; + std::memcpy(bytes.data() + k_offset, &k, sizeof(k)); + check_rejected(bytes); + } + + SECTION("levels beyond the input") { + auto bytes = three_levels.serialize(); + bytes[num_levels_offset] = 40; + bytes.resize(levels_offset); + REQUIRE_THROWS_AS(kll_sketch::deserialize(bytes.data(), bytes.size()), std::out_of_range); + std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); + s.write(reinterpret_cast(bytes.data()), bytes.size()); + REQUIRE_THROWS_AS(kll_sketch::deserialize(s), std::runtime_error); + } +} + } /* namespace datasketches */ From 43f82aaac778d3368bc7f7547ab50812f504fb25 Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Fri, 30 Oct 2026 13:51:04 -0700 Subject: [PATCH 2/5] REQ: check k and the compactor headers when deserializing deserialize() used the compactor header fields without checking them. A section size or number of sections of zero gave a nominal capacity of zero, so the buffer did not grow on the next update. The bytes overload checked 8 of the 20 header bytes. Large lg_weight and number of sections values made shifts undefined, and a non-empty sketch with no items read past the end of its only compactor. Both overloads now check: - k is even and in [MIN_K, MAX_K] - a non-empty sketch has at least one level, and exactly one with raw items, and a single level holds at least one item - compactor i has lg_weight i - the section size rounds to an even value in [MIN_K, k] - the number of sections is in [1, 64], and the raw section size, rounded to an integer, times the number of sections is at most k * k, as in datasketches-java The bytes overload checks the input holds the full compactor header. Co-Authored-By: Claude Opus 5.5 --- req/include/req_compactor.hpp | 5 +- req/include/req_compactor_impl.hpp | 34 ++++++- req/include/req_sketch.hpp | 2 + req/include/req_sketch_impl.hpp | 29 +++++- req/test/req_sketch_test.cpp | 143 +++++++++++++++++++++++++++++ 5 files changed, 206 insertions(+), 7 deletions(-) diff --git a/req/include/req_compactor.hpp b/req/include/req_compactor.hpp index f013e49d..87ddf1b3 100755 --- a/req/include/req_compactor.hpp +++ b/req/include/req_compactor.hpp @@ -87,11 +87,11 @@ class req_compactor { template static req_compactor deserialize(std::istream& is, const S& serde, const Comparator& comparator, - const Allocator& allocator, bool sorted, bool hra); + const Allocator& allocator, bool sorted, bool hra, uint16_t k, uint8_t level); template static std::pair deserialize(const void* bytes, size_t size, const S& serde, - const Comparator& comparator, const Allocator& allocator, bool sorted, bool hra); + const Comparator& comparator, const Allocator& allocator, bool sorted, bool hra, uint16_t k, uint8_t level); template static req_compactor deserialize(std::istream& is, const S& serde, const Comparator& comparator, @@ -122,6 +122,7 @@ class req_compactor { void ensure_space(uint32_t num); static uint32_t nearest_even(float value); + static void check_header(uint16_t k, uint8_t level, uint8_t lg_weight, float section_size_raw, uint8_t num_sections); template static void promote_evens_or_odds(InIter from, InIter to, bool flag, OutIter dst); diff --git a/req/include/req_compactor_impl.hpp b/req/include/req_compactor_impl.hpp index 72aafc94..bcc276c6 100755 --- a/req/include/req_compactor_impl.hpp +++ b/req/include/req_compactor_impl.hpp @@ -326,6 +326,30 @@ uint32_t req_compactor::nearest_even(float value) { return static_cast(round(value / 2)) << 1; } +template +void req_compactor::check_header(uint16_t k, uint8_t level, uint8_t lg_weight, float section_size_raw, + uint8_t num_sections) { + // the sketch creates the compactor at level h with lg_weight h + if (lg_weight != level) { + throw std::invalid_argument("Possible corruption: compactor " + std::to_string(level) + " has lg_weight " + + std::to_string(lg_weight)); + } + // the section size starts at k and shrinks to no less than MIN_K + // (the first check is false for NaN and keeps nearest_even() in range) + const uint32_t section_size = (section_size_raw > 0 && section_size_raw <= k + 1) ? nearest_even(section_size_raw) : 0; + if (section_size < req_constants::MIN_K || section_size > k) { + throw std::invalid_argument("Possible corruption: section size " + std::to_string(section_size_raw) + + " is out of range for k " + std::to_string(k)); + } + // the number of sections starts at 3 and doubles, and must keep 1 << (num_sections - 1) defined + // the limit uses the raw section size: when that has shrunk to 3, the even section size is 4 and can exceed it + const uint32_t rounded_section_size = static_cast(round(section_size_raw)); + if (num_sections < 1 || num_sections > 64 || rounded_section_size * num_sections > static_cast(k) * k) { + throw std::invalid_argument("Possible corruption: number of sections " + std::to_string(num_sections) + + " is out of range for section size " + std::to_string(section_size_raw) + " and k " + std::to_string(k)); + } +} + template template void req_compactor::promote_evens_or_odds(InIter from, InIter to, bool odds, OutIter dst) { @@ -394,13 +418,15 @@ size_t req_compactor::serialize(void* dst, size_t capacity, const S& se template template req_compactor req_compactor::deserialize(std::istream& is, const S& serde, - const C& comparator, const A& allocator, bool sorted, bool hra) { + const C& comparator, const A& allocator, bool sorted, bool hra, uint16_t k, uint8_t level) { auto state = read(is); auto section_size_raw = read(is); auto lg_weight = read(is); auto num_sections = read(is); read(is); // padding auto num_items = read(is); + if (!is.good()) throw std::runtime_error("error reading from std::istream"); + check_header(k, level, lg_weight, section_size_raw, num_sections); auto items = deserialize_items(is, serde, allocator, num_items); return req_compactor(hra, lg_weight, sorted, section_size_raw, num_sections, state, std::move(items), num_items, comparator, allocator); @@ -431,8 +457,9 @@ auto req_compactor::deserialize_items(std::istream& is, const S& serde, template template std::pair, size_t> req_compactor::deserialize(const void* bytes, size_t size, - const S& serde, const C& comparator, const A& allocator, bool sorted, bool hra) { - ensure_minimum_memory(size, 8); + const S& serde, const C& comparator, const A& allocator, bool sorted, bool hra, uint16_t k, uint8_t level) { + ensure_minimum_memory(size, sizeof(state_) + sizeof(section_size_raw_) + sizeof(lg_weight_) + sizeof(num_sections_) + + sizeof(uint16_t) + sizeof(num_items_)); const char* ptr = static_cast(bytes); const char* end_ptr = static_cast(bytes) + size; @@ -447,6 +474,7 @@ std::pair, size_t> req_compactor::deserialize(co ptr += 2; // padding uint32_t num_items; ptr += copy_from_mem(ptr, num_items); + check_header(k, level, lg_weight, section_size_raw, num_sections); auto pair = deserialize_items(ptr, end_ptr - ptr, serde, allocator, num_items); ptr += pair.second; return std::pair( diff --git a/req/include/req_sketch.hpp b/req/include/req_sketch.hpp index 2e2bc309..ed6d11f7 100755 --- a/req/include/req_sketch.hpp +++ b/req/include/req_sketch.hpp @@ -465,7 +465,9 @@ class req_sketch { optional&& min_item, optional&& max_item, std::vector&& compactors, const Comparator& comparator); + static void check_k(uint16_t k); static void check_preamble_ints(uint8_t preamble_ints, uint8_t num_levels); + static void check_num_levels(uint8_t num_levels, bool raw_items); static void check_serial_version(uint8_t serial_version); static void check_family_id(uint8_t family_id); diff --git a/req/include/req_sketch_impl.hpp b/req/include/req_sketch_impl.hpp index 8acea8bc..3c1cb408 100755 --- a/req/include/req_sketch_impl.hpp +++ b/req/include/req_sketch_impl.hpp @@ -456,6 +456,7 @@ req_sketch req_sketch::deserialize(std::istream& is, const Ser const auto num_levels = read(is); const auto num_raw_items = read(is); + check_k(k); check_preamble_ints(preamble_ints, num_levels); check_serial_version(serial_version); check_family_id(family_id); @@ -469,6 +470,7 @@ req_sketch req_sketch::deserialize(std::istream& is, const Ser optional max_item; const bool raw_items = flags_byte & (1 << flags::RAW_ITEMS); + check_num_levels(num_levels, raw_items); const bool is_level_0_sorted = flags_byte & (1 << flags::IS_LEVEL_ZERO_SORTED); std::vector compactors(allocator); @@ -494,10 +496,12 @@ req_sketch req_sketch::deserialize(std::istream& is, const Ser compactors.push_back(Compactor::deserialize(is, sd, comparator, allocator, is_level_0_sorted, k, num_raw_items, hra)); } else { for (size_t i = 0; i < num_levels; ++i) { - compactors.push_back(Compactor::deserialize(is, sd, comparator, allocator, i == 0 ? is_level_0_sorted : true, hra)); + compactors.push_back(Compactor::deserialize(is, sd, comparator, allocator, i == 0 ? is_level_0_sorted : true, hra, + k, static_cast(i))); } } if (num_levels == 1) { + if (compactors[0].get_num_items() == 0) throw std::invalid_argument("Possible corruption: non-empty sketch with no items"); const auto begin = compactors[0].begin(); const auto end = compactors[0].end(); n = compactors[0].get_num_items(); @@ -537,6 +541,7 @@ req_sketch req_sketch::deserialize(const void* bytes, size_t s uint8_t num_raw_items; ptr += copy_from_mem(ptr, num_raw_items); + check_k(k); check_preamble_ints(preamble_ints, num_levels); check_serial_version(serial_version); check_family_id(family_id); @@ -549,6 +554,7 @@ req_sketch req_sketch::deserialize(const void* bytes, size_t s optional max_item; const bool raw_items = flags_byte & (1 << flags::RAW_ITEMS); + check_num_levels(num_levels, raw_items); const bool is_level_0_sorted = flags_byte & (1 << flags::IS_LEVEL_ZERO_SORTED); std::vector compactors(allocator); @@ -577,12 +583,14 @@ req_sketch req_sketch::deserialize(const void* bytes, size_t s ptr += pair.second; } else { for (size_t i = 0; i < num_levels; ++i) { - auto pair = Compactor::deserialize(ptr, end_ptr - ptr, sd, comparator, allocator, i == 0 ? is_level_0_sorted : true, hra); + auto pair = Compactor::deserialize(ptr, end_ptr - ptr, sd, comparator, allocator, i == 0 ? is_level_0_sorted : true, hra, + k, static_cast(i)); compactors.push_back(std::move(pair.first)); ptr += pair.second; } } if (num_levels == 1) { + if (compactors[0].get_num_items() == 0) throw std::invalid_argument("Possible corruption: non-empty sketch with no items"); const auto begin = compactors[0].begin(); const auto end = compactors[0].end(); n = compactors[0].get_num_items(); @@ -706,6 +714,23 @@ sorted_view_(nullptr) update_num_retained(); } +template +void req_sketch::check_k(uint16_t k) { + if (k < req_constants::MIN_K || k > req_constants::MAX_K || (k & 1) != 0) { + throw std::invalid_argument("Possible corruption: k must be even and in the range [" + + std::to_string(req_constants::MIN_K) + ", " + std::to_string(req_constants::MAX_K) + "]: " + std::to_string(k)); + } +} + +template +void req_sketch::check_num_levels(uint8_t num_levels, bool raw_items) { + // a non-empty sketch has at least one compactor, and raw items fit in one + if (num_levels == 0 || (raw_items && num_levels != 1)) { + throw std::invalid_argument("Possible corruption: number of levels " + std::to_string(num_levels) + + (raw_items ? " with raw items" : "")); + } +} + template void req_sketch::check_preamble_ints(uint8_t preamble_ints, uint8_t num_levels) { const uint8_t expected_preamble_ints = num_levels > 1 ? 4 : 2; diff --git a/req/test/req_sketch_test.cpp b/req/test/req_sketch_test.cpp index c8cf5cbe..6cff32b9 100755 --- a/req/test/req_sketch_test.cpp +++ b/req/test/req_sketch_test.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -592,4 +593,146 @@ TEST_CASE("get_rank equivalence") { // sketch.serialize(os); //} + +template +static void put_value(Bytes& bytes, size_t offset, V value) { + std::memcpy(bytes.data() + offset, &value, sizeof(value)); +} + +TEST_CASE("req sketch: deserialize rejects inconsistent compactors", "[req_sketch]") { + using vector_bytes = req_sketch::vector_bytes; + // preamble: k at 4, number of levels at 6, number of raw items at 7 + // compactor: state(8), section size(4), lg_weight(1), number of sections(1), padding(2), number of items(4), items + const size_t k_offset = 4; + const size_t num_levels_offset = 6; + const size_t num_raw_items_offset = 7; + const size_t estimation_compactor = 24; // after the preamble, n, min and max + const size_t exact_compactor = 8; // after the preamble + const size_t section_size = 8; + const size_t lg_weight = 12; + const size_t num_sections = 13; + const size_t num_items = 16; + const size_t items = 20; + auto check_rejected = [](const vector_bytes& bytes) { + REQUIRE_THROWS_AS(req_sketch::deserialize(bytes.data(), bytes.size()), std::invalid_argument); + std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); + s.write(reinterpret_cast(bytes.data()), bytes.size()); + REQUIRE_THROWS_AS(req_sketch::deserialize(s), std::invalid_argument); + }; + + req_sketch raw(12); + for (int i = 0; i < 3; ++i) raw.update(static_cast(i)); + req_sketch exact(12); + for (int i = 0; i < 50; ++i) exact.update(static_cast(i)); + req_sketch estimation(12); + for (int i = 0; i < 1000; ++i) estimation.update(static_cast(i)); + REQUIRE(estimation.serialize()[num_levels_offset] > 2); + + SECTION("valid images") { + // sketches deep enough for the sections to have grown, in both rank accuracy modes + for (bool hra: {true, false}) { + req_sketch sketch(12, hra); + for (int i = 0; i < 1000000; ++i) sketch.update(static_cast(i)); + auto bytes = sketch.serialize(); + REQUIRE(req_sketch::deserialize(bytes.data(), bytes.size()).get_n() == sketch.get_n()); + std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); + sketch.serialize(s); + REQUIRE(req_sketch::deserialize(s).get_n() == sketch.get_n()); + } + } + + SECTION("valid images with the smallest section size") { + // the raw section size has shrunk to 3, which the section size rounds up to 4 + req_sketch sketch(6); + for (int i = 0; i < 100000; ++i) sketch.update(static_cast(i)); + auto bytes = sketch.serialize(); + float raw_section_size; + std::memcpy(&raw_section_size, bytes.data() + estimation_compactor + section_size, sizeof(raw_section_size)); + REQUIRE(raw_section_size == Approx(3).margin(0.001)); + REQUIRE(bytes[estimation_compactor + num_sections] == 12); + REQUIRE(req_sketch::deserialize(bytes.data(), bytes.size()).get_n() == sketch.get_n()); + std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); + sketch.serialize(s); + REQUIRE(req_sketch::deserialize(s).get_n() == sketch.get_n()); + + // k = 12 needs about 2 billion items to get here, so set the header it would have + bytes = estimation.serialize(); + put_value(bytes, estimation_compactor + section_size, 3.0f); + put_value(bytes, estimation_compactor + num_sections, static_cast(48)); + REQUIRE(req_sketch::deserialize(bytes.data(), bytes.size()).get_n() == estimation.get_n()); + } + + SECTION("invalid k") { + for (uint16_t k: {2, 13, 2000}) { + auto bytes = estimation.serialize(); + put_value(bytes, k_offset, k); + check_rejected(bytes); + } + } + + SECTION("section size out of range") { + for (float size: {0.0f, 2.0f, 14.0f, 1e12f, std::numeric_limits::quiet_NaN()}) { + auto bytes = estimation.serialize(); + put_value(bytes, estimation_compactor + section_size, size); + check_rejected(bytes); + } + } + + SECTION("number of sections out of range") { + for (uint8_t sections: {0, 65}) { + auto bytes = estimation.serialize(); + put_value(bytes, estimation_compactor + num_sections, sections); + check_rejected(bytes); + } + // in range, but too many for this section size and k + auto bytes = estimation.serialize(); + put_value(bytes, estimation_compactor + section_size, 3.0f); + put_value(bytes, estimation_compactor + num_sections, static_cast(64)); + check_rejected(bytes); + } + + SECTION("wrong lg_weight") { + for (uint8_t weight: {1, 70}) { + auto bytes = estimation.serialize(); + put_value(bytes, estimation_compactor + lg_weight, weight); + check_rejected(bytes); + } + auto bytes = exact.serialize(); + put_value(bytes, exact_compactor + lg_weight, static_cast(1)); + check_rejected(bytes); + // the second compactor repeats the first one's lg_weight + bytes = estimation.serialize(); + uint32_t count; + std::memcpy(&count, bytes.data() + estimation_compactor + num_items, sizeof(count)); + const size_t second_compactor = estimation_compactor + items + count * sizeof(float); + REQUIRE(bytes[second_compactor + lg_weight] == 1); + put_value(bytes, second_compactor + lg_weight, static_cast(0)); + check_rejected(bytes); + } + + SECTION("no levels") { + auto bytes = exact.serialize(); + bytes[num_levels_offset] = 0; + check_rejected(bytes); + bytes = raw.serialize(); + bytes[num_levels_offset] = 0; + check_rejected(bytes); + } + + SECTION("no items") { + auto bytes = exact.serialize(); + put_value(bytes, exact_compactor + num_items, static_cast(0)); + check_rejected(bytes); + bytes = raw.serialize(); + bytes[num_raw_items_offset] = 0; + check_rejected(bytes); + } + + SECTION("compactor header beyond the input") { + auto bytes = estimation.serialize(); + bytes.resize(estimation_compactor + 10); + REQUIRE_THROWS_AS(req_sketch::deserialize(bytes.data(), bytes.size()), std::out_of_range); + } +} + } /* namespace datasketches */ From 6a7831777d23ebd0b554d21b36f2bba9dca6a062 Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Fri, 30 Oct 2026 13:51:04 -0700 Subject: [PATCH 3/5] Quantiles: check the input size before the serial version 1 field The bytes overload of deserialize() read the unused 8-byte field of serial version 1 images without checking that the input holds it. Co-Authored-By: Claude Opus 5.5 --- quantiles/include/quantiles_sketch_impl.hpp | 1 + quantiles/test/quantiles_sketch_test.cpp | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/quantiles/include/quantiles_sketch_impl.hpp b/quantiles/include/quantiles_sketch_impl.hpp index 2dacf21e..a3466f3a 100644 --- a/quantiles/include/quantiles_sketch_impl.hpp +++ b/quantiles/include/quantiles_sketch_impl.hpp @@ -531,6 +531,7 @@ auto quantiles_sketch::deserialize(const void* bytes, size_t size, cons if (serial_version == 1) { uint64_t unused_long; + ensure_minimum_memory(end_ptr - ptr, sizeof(unused_long)); ptr += copy_from_mem(ptr, unused_long); // no longer used } diff --git a/quantiles/test/quantiles_sketch_test.cpp b/quantiles/test/quantiles_sketch_test.cpp index 8230f6a1..0a23a7c7 100644 --- a/quantiles/test/quantiles_sketch_test.cpp +++ b/quantiles/test/quantiles_sketch_test.cpp @@ -956,4 +956,17 @@ TEST_CASE("quantiles sketch", "[quantiles_sketch]") { } } + +TEST_CASE("quantiles sketch: deserialize serial version 1 truncated after min and max", "[quantiles_sketch]") { + quantiles_sketch sketch(16); + for (int i = 0; i < 1000; ++i) sketch.update(i); + auto bytes = sketch.serialize(); + // serial version 1 has an unused 8-byte field after min and max + bytes[0] = 5; // preamble longs + bytes[1] = 1; // serial version + bytes[3] &= ~(1 << 3); // not compact + bytes.resize(8 + sizeof(uint64_t) + 2 * sizeof(int)); // preamble, n, min and max + REQUIRE_THROWS_AS(quantiles_sketch::deserialize(bytes.data(), bytes.size()), std::out_of_range); +} + } /* namespace datasketches */ From d0b7833559bbb81dc0a0f4d8dda2590b5846082a Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Thu, 5 Nov 2026 15:59:18 -0800 Subject: [PATCH 4/5] NOTICE: ASF copyright starts in 2019, Yahoo range ends in 2025 Co-Authored-By: Claude Opus 5.5 --- NOTICE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NOTICE b/NOTICE index 14c1ff98..f3a8a1bc 100644 --- a/NOTICE +++ b/NOTICE @@ -1,9 +1,9 @@ Apache DataSketches C++ -Copyright 2025-2026 The Apache Software Foundation +Copyright 2019-2026 The Apache Software Foundation Copyright 2015-2018 Yahoo Inc. Copyright 2019-2020 Verizon Media -Copyright 2021- Yahoo Inc. +Copyright 2021-2025 Yahoo Inc. This product includes software developed at The Apache Software Foundation (http://www.apache.org/). From ad1a3615f5cb4d40b67387b959f35af8d52c47ff Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Fri, 6 Nov 2026 14:22:49 -0800 Subject: [PATCH 5/5] REQ test: set the smallest section size in the k = 6 header The test relied on a real sketch shrinking its raw section size to 3. Setting the header tests the same image without depending on how far the section size shrinks. Co-Authored-By: Claude Opus 5.5 --- req/test/req_sketch_test.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/req/test/req_sketch_test.cpp b/req/test/req_sketch_test.cpp index 6cff32b9..99e59140 100755 --- a/req/test/req_sketch_test.cpp +++ b/req/test/req_sketch_test.cpp @@ -642,20 +642,19 @@ TEST_CASE("req sketch: deserialize rejects inconsistent compactors", "[req_sketc } SECTION("valid images with the smallest section size") { - // the raw section size has shrunk to 3, which the section size rounds up to 4 + // the raw section size can have shrunk to 3, which the section size rounds up to 4 + // older versions wrote such images, so set the header they would have req_sketch sketch(6); for (int i = 0; i < 100000; ++i) sketch.update(static_cast(i)); auto bytes = sketch.serialize(); - float raw_section_size; - std::memcpy(&raw_section_size, bytes.data() + estimation_compactor + section_size, sizeof(raw_section_size)); - REQUIRE(raw_section_size == Approx(3).margin(0.001)); - REQUIRE(bytes[estimation_compactor + num_sections] == 12); + put_value(bytes, estimation_compactor + section_size, 3.0f); + put_value(bytes, estimation_compactor + num_sections, static_cast(12)); REQUIRE(req_sketch::deserialize(bytes.data(), bytes.size()).get_n() == sketch.get_n()); std::stringstream s(std::ios::in | std::ios::out | std::ios::binary); - sketch.serialize(s); + s.write(reinterpret_cast(bytes.data()), bytes.size()); REQUIRE(req_sketch::deserialize(s).get_n() == sketch.get_n()); - // k = 12 needs about 2 billion items to get here, so set the header it would have + // the same for k = 12 bytes = estimation.serialize(); put_value(bytes, estimation_compactor + section_size, 3.0f); put_value(bytes, estimation_compactor + num_sections, static_cast(48));