Skip to content

Commit

Permalink
fix: another segfault in 64 bit deserialize_safe (#664)
Browse files Browse the repository at this point in the history
actually update the previous_high32 variable to ensure the ghigh keys are
actually strictly increasing
  • Loading branch information
Dr-Emann authored Sep 20, 2024
1 parent 032450e commit f0e8f32
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/roaring64.c
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,8 @@ roaring64_bitmap_t *roaring64_bitmap_portable_deserialize_safe(
roaring64_bitmap_free(r);
return NULL;
}
previous_high32 = high32;

// Read the 32-bit Roaring bitmaps representing the least significant
// bits of a set of elements.
size_t bitmap32_size = roaring_bitmap_portable_deserialize_size(
Expand Down
53 changes: 52 additions & 1 deletion tests/roaring64_serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ DEFINE_TEST(test_64deseroverlappingkeys) {
// Container count - 1
1, 0,
// Run Flag Bitset (no runs)
0, 0,
0,
// Upper 16 bits of the first container
0, 0,
// Cardinality - 1 of the first container
Expand Down Expand Up @@ -125,6 +125,56 @@ DEFINE_TEST(test_64deseroverlappingkeys) {
roaring64_bitmap_free(r);
}
}

DEFINE_TEST(test_64deseroverlappingupper32) {
// Two bitmaps, with a single array container each, at the same key (0).
// clang-format off
char simple_bitmap[] = {
// Number of 32 bit bitmaps
2, 0, 0, 0, 0, 0, 0, 0,
// Top 32 bits of the first bitmap
0, 0, 0, 0,
// Serial Cookie
0x3B, 0x30,
// Container count - 1
0, 0,
// Run Flag Bitset (not a run)
0,
// Upper 16 bits of the only container
0, 0,
// Cardinality - 1 of the only container
0, 0,
// Only value of first container
0, 0,
// Top 32 bits of the second bitmap
0, 0, 0, 0,
// Serial Cookie
0x3B, 0x30,
// Container count - 1
0, 0,
// Run Flag Bitset (not a run)
0,
// Upper 16 bits of the only container
0, 0,
// Cardinality - 1 of the only container
0, 0,
// Only value of only container
0, 0,
};
// clang-format on

roaring64_bitmap_t* r = roaring64_bitmap_portable_deserialize_safe(
simple_bitmap, sizeof(simple_bitmap));
const char* reason = nullptr;
if (r != nullptr) {
if (roaring64_bitmap_internal_validate(r, &reason)) {
fail_msg(
"Validation must fail if a bitmap was returned, duplicate keys "
"are not allowed.");
}
roaring64_bitmap_free(r);
}
}
} // namespace

int main() {
Expand All @@ -142,6 +192,7 @@ int main() {
cmocka_unit_test(test_64mapsizetoosmall),
cmocka_unit_test(test_64mapspreadvals),
cmocka_unit_test(test_64deseroverlappingkeys),
cmocka_unit_test(test_64deseroverlappingupper32),
};
return cmocka_run_group_tests(tests, NULL, NULL);
#endif // CROARING_IS_BIG_ENDIAN
Expand Down

0 comments on commit f0e8f32

Please sign in to comment.