Caught a bug in roaring_bitmap_internal_validate

RoaringBitmap · Sep 27, 2023 · 70a927a · 70a927a
1 parent 1ade8ee
commit 70a927a
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 3 deletions.
diff --git a/include/roaring/portability.h b/include/roaring/portability.h
@@ -225,8 +225,8 @@ inline int roaring_leading_zeroes(unsigned long long input_num) {
 #ifndef CROARING_INTRINSICS
 #define CROARING_INTRINSICS 1
 #define roaring_unreachable __builtin_unreachable()
-static inline int roaring_trailing_zeroes(unsigned long long input_num) { return __builtin_ctzll(input_num); }
-static inline int roaring_leading_zeroes(unsigned long long input_num) { return __builtin_clzll(input_num); }
+inline int roaring_trailing_zeroes(unsigned long long input_num) { return __builtin_ctzll(input_num); }
+inline int roaring_leading_zeroes(unsigned long long input_num) { return __builtin_clzll(input_num); }
 #endif
 
 #if CROARING_REGULAR_VISUAL_STUDIO

diff --git a/src/containers/run.c b/src/containers/run.c
@@ -710,7 +710,10 @@ bool run_container_validate(const run_container_t *run, const char **reason) {
             *reason = "run start + length overflow";
             return false;
         }
-
+        if (end > (1<<16)) {
+            *reason = "run start + length too large";
+            return false;
+        }
         if (start < last_end) {
             *reason = "run start less than last end";
             return false;

diff --git a/tests/toplevel_unit.c b/tests/toplevel_unit.c
@@ -4595,10 +4595,47 @@ DEFINE_TEST(convert_to_bitset) {
     roaring_bitmap_free(r1);
 }
 
+
+bool deserialization_test(const char *data, size_t size) {
+    // We test that deserialization never fails.
+    roaring_bitmap_t *bitmap =
+        roaring_bitmap_portable_deserialize_safe(data, size);
+    if (bitmap) {
+        // The bitmap may not be usable if it does not follow the specification.
+        // We can validate the bitmap we recovered to make sure it is proper.
+        const char *reason_failure = NULL;
+        if (roaring_bitmap_internal_validate(bitmap, &reason_failure)) {
+            // the bitmap is ok!
+            uint32_t cardinality = roaring_bitmap_get_cardinality(bitmap);
+
+            for (uint32_t i = 100; i < 1000; i++) {
+                if (!roaring_bitmap_contains(bitmap, i)) {
+                    cardinality++;
+                    roaring_bitmap_add(bitmap, i);
+                }
+            }
+
+            uint32_t new_cardinality = roaring_bitmap_get_cardinality(bitmap);
+            if (cardinality != new_cardinality) {
+                return false;
+            }
+        }
+        roaring_bitmap_free(bitmap);
+    }
+    return true;
+}
+
+DEFINE_TEST(robust_deserialization) {
+    assert_true(deserialization_test(NULL, 0));
+    const char* test1 = "\x3b\x30\x00\x00\x01\x00\x00\xfa\x2e\x01\x00\x00\x02\xff\xff";
+    assert_true(deserialization_test(test1, 15));
+}
+
 int main() {
     tellmeall();
 
     const struct CMUnitTest tests[] = {
+        cmocka_unit_test(robust_deserialization),
         cmocka_unit_test(issue457),
         cmocka_unit_test(convert_to_bitset),
         cmocka_unit_test(issue440),