[incubator-datasketches-cpp] 06/07: add bounds checks to frequent items

[jm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jmalkin pushed a commit to branch check_buffer_bounds
in repository https://gitbox.apache.org/repos/asf/incubator-datasketches-cpp.git

commit f67102e63f72280dbf586b9e856caf5d5588b04b
Author: Jon Malkin <jm...@users.noreply.github.com>
AuthorDate: Tue Apr 28 16:36:08 2020 -0700

    add bounds checks to frequent items
---
 common/test/test_type.hpp                 |  2 +-
 fi/include/frequent_items_sketch_impl.hpp | 15 ++++++++++++---
 sampling/include/var_opt_sketch_impl.hpp  |  2 +-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/common/test/test_type.hpp b/common/test/test_type.hpp
index cf4a44f..abbd8f3 100644
--- a/common/test/test_type.hpp
+++ b/common/test/test_type.hpp
@@ -88,7 +88,7 @@ struct test_type_serde {
       new (&items[i]) test_type(value);
     }
   }
-  size_t size_of_item(const test_type& item) {
+  size_t size_of_item(const test_type&) {
     return sizeof(int);
   }
 };
diff --git a/fi/include/frequent_items_sketch_impl.hpp b/fi/include/frequent_items_sketch_impl.hpp
index c3a40b8..40a70f5 100644
--- a/fi/include/frequent_items_sketch_impl.hpp
+++ b/fi/include/frequent_items_sketch_impl.hpp
@@ -24,6 +24,8 @@
 #include <cstring>
 #include <limits>
 
+#include "memory_operations.hpp"
+
 namespace datasketches {
 
 // clang++ seems to require this declaration for CMAKE_BUILD_TYPE='Debug"
@@ -209,6 +211,7 @@ vector_u8<A> frequent_items_sketch<T, W, H, E, S, A>::serialize(unsigned header_
   const size_t size = header_size_bytes + get_serialized_size_bytes();
   vector_u8<A> bytes(size);
   uint8_t* ptr = bytes.data() + header_size_bytes;
+  uint8_t* end_ptr = ptr + size;
 
   const uint8_t preamble_longs = is_empty() ? PREAMBLE_LONGS_EMPTY : PREAMBLE_LONGS_NONEMPTY;
   ptr += copy_to_mem(&preamble_longs, ptr, sizeof(uint8_t));
@@ -245,7 +248,8 @@ vector_u8<A> frequent_items_sketch<T, W, H, E, S, A>::serialize(unsigned header_
     }
     ptr += copy_to_mem(weights, ptr, sizeof(W) * num_items);
     AllocW().deallocate(weights, num_items);
-    ptr += S().serialize(ptr, items, num_items);
+    const size_t bytes_remaining = end_ptr - ptr;
+    ptr += S().serialize(ptr, bytes_remaining, items, num_items);
     for (unsigned i = 0; i < num_items; i++) items[i].~T();
     A().deallocate(items, num_items);
   }
@@ -308,7 +312,9 @@ frequent_items_sketch<T, W, H, E, S, A> frequent_items_sketch<T, W, H, E, S, A>:
 
 template<typename T, typename W, typename H, typename E, typename S, typename A>
 frequent_items_sketch<T, W, H, E, S, A> frequent_items_sketch<T, W, H, E, S, A>::deserialize(const void* bytes, size_t size) {
+  ensure_minimum_memory(size, 8);
   const char* ptr = static_cast<const char*>(bytes);
+  const char* base = static_cast<const char*>(bytes);
   uint8_t preamble_longs;
   ptr += copy_from_mem(ptr, &preamble_longs, sizeof(uint8_t));
   uint8_t serial_version;
@@ -330,6 +336,7 @@ frequent_items_sketch<T, W, H, E, S, A> frequent_items_sketch<T, W, H, E, S, A>:
   check_serial_version(serial_version);
   check_family_id(family_id);
   check_size(lg_cur_size, lg_max_size);
+  ensure_minimum_memory(size, 1 << preamble_longs);
 
   frequent_items_sketch<T, W, H, E, S, A> sketch(lg_max_size, lg_cur_size);
   if (!is_empty) {
@@ -345,9 +352,11 @@ frequent_items_sketch<T, W, H, E, S, A> frequent_items_sketch<T, W, H, E, S, A>:
     // batch deserialization with intermediate array of items and weights
     typedef typename std::allocator_traits<A>::template rebind_alloc<W> AllocW;
     W* weights = AllocW().allocate(num_items);
+    ensure_minimum_memory(size, ptr - base + (sizeof(W) * num_items));
     ptr += copy_from_mem(ptr, weights, sizeof(W) * num_items);
     T* items = A().allocate(num_items);
-    ptr += S().deserialize(ptr, items, num_items);
+    const size_t bytes_remaining = size - (ptr - base);
+    ptr += S().deserialize(ptr, bytes_remaining, items, num_items);
     for (uint32_t i = 0; i < num_items; i++) {
       sketch.update(std::move(items[i]), weights[i]);
       items[i].~T();
@@ -436,7 +445,7 @@ void frequent_items_sketch<T, W, H, E, S, A>::check_weight(WW weight) {
 // version for integral unsigned type - no-op
 template<typename T, typename W, typename H, typename E, typename S, typename A>
 template<typename WW, typename std::enable_if<std::is_integral<WW>::value && std::is_unsigned<WW>::value, int>::type>
-void frequent_items_sketch<T, W, H, E, S, A>::check_weight(WW weight) {}
+void frequent_items_sketch<T, W, H, E, S, A>::check_weight(WW) {}
 
 // version for floating point type
 template<typename T, typename W, typename H, typename E, typename S, typename A>
diff --git a/sampling/include/var_opt_sketch_impl.hpp b/sampling/include/var_opt_sketch_impl.hpp
index 6974dba..4292d08 100644
--- a/sampling/include/var_opt_sketch_impl.hpp
+++ b/sampling/include/var_opt_sketch_impl.hpp
@@ -363,7 +363,7 @@ var_opt_sketch<T,S,A>::var_opt_sketch(uint32_t k, resize_factor rf, bool is_gadg
 
   // read the sample items, skipping the gap. Either h_ or r_ may be 0
   size_t bytes_remaining = end_ptr - ptr;
-  ptr += S().deserialize(ptr, bytes_remaining, data_, h_); // ala data_[0]
+  ptr += S().deserialize(ptr, bytes_remaining, data_, h_);
   bytes_remaining = end_ptr - ptr;
   ptr += S().deserialize(ptr, bytes_remaining, &data_[h_ + 1], r_);
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org