Serialized bitmaps with run bitmaps should have bitmaps (#778)

* Serialized bitmaps with run bitmaps should have bitmaps

* lint

* lint

* lint

* updating cifuzz

Author: lemire

.github/workflows/cifuzz.yml

@@ -8,12 +8,12 @@ jobs:
     steps:
     - name: Build Fuzzers
       id: build
-      uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@d318097b285bc695f785b98d40c2d058c0f438b5 # master
+      uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master
       with:
         oss-fuzz-project-name: 'croaring'
         dry-run: false
     - name: Run Fuzzers
-      uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@d318097b285bc695f785b98d40c2d058c0f438b5 # master
+      uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
       with:
         oss-fuzz-project-name: 'croaring'
         fuzz-seconds: 300

cpp/roaring/roaring.hh

@@ -686,6 +686,15 @@ class Roaring {
         return Roaring(r);
     }
 
+    /**
+     * Compute how many bytes would be read by readSafe.  Returns 0 if the
+     * serialized data is invalid.
+     * This is meant to be compatible with the Java and Go versions.
+     */
+    static size_t serializedSizeInBytesSafe(const char *buf, size_t maxbytes) {
+        return api::roaring_bitmap_portable_deserialize_size(buf, maxbytes);
+    }
+
     /**
      * How many bytes are required to serialize this bitmap (meant to be
      * compatible with Java and Go versions)

cpp/roaring/roaring64map.hh

@@ -1204,6 +1204,9 @@ class Roaring64Map {
             ROARING_TERMINATE("ran out of bytes");
         }
         Roaring64Map result;
+        if (maxbytes < sizeof(uint64_t)) {
+            ROARING_TERMINATE("ran out of bytes");
+        }
         uint64_t map_size;
         std::memcpy(&map_size, buf, sizeof(uint64_t));
         buf += sizeof(uint64_t);
@@ -1219,11 +1222,15 @@ class Roaring64Map {
             buf += sizeof(uint32_t);
             maxbytes -= sizeof(uint32_t);
             // read map value Roaring
+            size_t needed_bytes =
+                Roaring::serializedSizeInBytesSafe(buf, maxbytes);
+            if (needed_bytes == 0) {
+                ROARING_TERMINATE("invalid serialized data");
+            }
             Roaring read_var = Roaring::readSafe(buf, maxbytes);
             // forward buffer past the last Roaring Bitmap
-            size_t tz = read_var.getSizeInBytes(true);
-            buf += tz;
-            maxbytes -= tz;
+            buf += needed_bytes;
+            maxbytes -= needed_bytes;
             result.emplaceOrInsert(key, std::move(read_var));
         }
         return result;

include/roaring/containers/containers.h

@@ -548,9 +548,9 @@ static inline container_t *container_remove(
 }
 
 /**
- * Check whether a value is in a container, requires a  typecode
+ * Check whether a value is in a container, requires a typecode
  */
-static inline bool container_contains(
+inline bool container_contains(
     const container_t *c, uint16_t val,
     uint8_t typecode  // !!! should be second argument?
 ) {

src/containers/containers.c

@@ -50,6 +50,10 @@ extern bool container_iterator_next(const container_t *c, uint8_t typecode,
 extern bool container_iterator_prev(const container_t *c, uint8_t typecode,
                                     roaring_container_iterator_t *it,
                                     uint16_t *value);
+extern bool container_contains(
+    const container_t *c, uint16_t val,
+    uint8_t typecode  // !!! should be second argument?
+);
 
 void container_free(container_t *c, uint8_t type) {
     switch (type) {

tests/cpp_random_unit.cpp

@@ -353,6 +353,21 @@ DEFINE_TEST(random_doublecheck_test) {
     }
 }
 
+#if ROARING_EXCEPTIONS
+// credit: Oleg Lazari
+DEFINE_TEST(safe_test_lazari) {
+    unsigned char payload[] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                               0x00, 0x00, 0x00, 0x00, 0x00, 0x3b, 0x30,
+                               0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                               0x00, 0x6a, 0x6a, 0xd4};
+    try {
+        roaring::Roaring64Map::readSafe((const char *)payload, sizeof(payload));
+        assert_false(true);  // it should not reach here
+    } catch (...) {
+    }
+}
+#endif
+
 DEFINE_TEST(random_doublecheck_test_64) {
     //
     // Make a group of bitsets to choose from when performing operations.
@@ -487,6 +502,9 @@ int main() {
     gravity64 = (static_cast<uint64_t>(rand()) << 32) + rand() % 20000 - 10000;
 
     const struct CMUnitTest tests[] = {
+#if ROARING_EXCEPTIONS
+        cmocka_unit_test(safe_test_lazari),
+#endif
         cmocka_unit_test(sanity_check_doublechecking),
         cmocka_unit_test(sanity_check_doublechecking_64),
         cmocka_unit_test(random_doublecheck_test),