Skip to content

Commit 796bdf9

Browse files
scsiguymeta-codesync[bot]
authored andcommitted
Additional binary index input validation (#4898)
Summary: Pull Request resolved: #4898 - read_binary_hash_invlists(): Protect against negative values that cause out of bounds reads after conversion from int to size_t. - read_binary_hash_invlists() and read_index_binary_up(): Prevent silent corruption of the deserialized index by ensuring BitstringReader::read() has a valid, positive value for nbit. Reviewed By: mnorris11 Differential Revision: D95839908 fbshipit-source-id: 86cfac4b1ff2df8b1834222b8cadebe083539217
1 parent d0434be commit 796bdf9

2 files changed

Lines changed: 67 additions & 0 deletions

File tree

faiss/impl/index_read.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,6 +1901,18 @@ static void read_binary_hash_invlists(
19011901
READ1(sz);
19021902
int il_nbit = 0;
19031903
READ1(il_nbit);
1904+
FAISS_THROW_IF_NOT_FMT(
1905+
il_nbit >= 0,
1906+
"invalid binary hash invlists il_nbit=%d (must be >= 0)",
1907+
il_nbit);
1908+
if (sz > 0) {
1909+
FAISS_THROW_IF_NOT_FMT(
1910+
il_nbit > 0,
1911+
"invalid binary hash invlists il_nbit=%d for sz=%zd "
1912+
"(must be > 0 when entries exist)",
1913+
il_nbit,
1914+
sz);
1915+
}
19041916
// buffer for bitstrings
19051917
size_t bits_per_entry = (size_t)b + (size_t)il_nbit;
19061918
size_t total_bits =
@@ -2013,6 +2025,10 @@ std::unique_ptr<IndexBinary> read_index_binary_up(IOReader* f, int io_flags) {
20132025
auto idxh = std::make_unique<IndexBinaryHash>();
20142026
read_index_binary_header(*idxh, f);
20152027
READ1(idxh->b);
2028+
FAISS_THROW_IF_NOT_FMT(
2029+
idxh->b > 0,
2030+
"invalid IndexBinaryHash b=%d (must be > 0)",
2031+
idxh->b);
20162032
READ1(idxh->nflip);
20172033
read_binary_hash_invlists(idxh->invlists, idxh->b, f);
20182034
idx = std::move(idxh);

tests/test_read_index_deserialize.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,3 +439,54 @@ TEST(ReadIndexDeserialize, BinaryHashEmptyInvlistBuffer) {
439439

440440
expect_binary_read_throws_with(buf, "binary hash invlists");
441441
}
442+
443+
// -----------------------------------------------------------------------
444+
// Test: IndexBinaryHash with b=0 triggers the b > 0 validation.
445+
// Without this check, BitstringReader::read(0) would silently produce
446+
// garbage hash values on every inverted-list entry.
447+
// -----------------------------------------------------------------------
448+
TEST(ReadIndexDeserialize, BinaryHashBZero) {
449+
std::vector<uint8_t> buf;
450+
push_fourcc(buf, "IBHh");
451+
push_binary_index_header(buf, /*d=*/16, /*ntotal=*/0);
452+
push_val<int>(buf, 0); // b = 0 (invalid)
453+
454+
expect_binary_read_throws_with(buf, "IndexBinaryHash b=");
455+
}
456+
457+
// -----------------------------------------------------------------------
458+
// Test: read_binary_hash_invlists with negative il_nbit triggers the
459+
// il_nbit >= 0 validation. Without this check, the negative value would
460+
// wrap to a huge size_t in the bits-per-entry calculation, causing an
461+
// out-of-bounds read in BitstringReader.
462+
// -----------------------------------------------------------------------
463+
TEST(ReadIndexDeserialize, BinaryHashNegativeIlNbit) {
464+
std::vector<uint8_t> buf;
465+
push_fourcc(buf, "IBHh");
466+
push_binary_index_header(buf, /*d=*/16, /*ntotal=*/0);
467+
push_val<int>(buf, 4); // b
468+
push_val<int>(buf, 0); // nflip
469+
// read_binary_hash_invlists fields:
470+
push_val<size_t>(buf, size_t(0)); // sz = 0
471+
push_val<int>(buf, -1); // il_nbit = -1 (invalid)
472+
473+
expect_binary_read_throws_with(buf, "il_nbit=");
474+
}
475+
476+
// -----------------------------------------------------------------------
477+
// Test: read_binary_hash_invlists with il_nbit=0 but sz > 0 triggers
478+
// the il_nbit > 0 validation. Without this check, every inverted-list
479+
// size would silently read as 0, corrupting the deserialized index.
480+
// -----------------------------------------------------------------------
481+
TEST(ReadIndexDeserialize, BinaryHashIlNbitZeroWithEntries) {
482+
std::vector<uint8_t> buf;
483+
push_fourcc(buf, "IBHh");
484+
push_binary_index_header(buf, /*d=*/16, /*ntotal=*/0);
485+
push_val<int>(buf, 4); // b
486+
push_val<int>(buf, 0); // nflip
487+
// read_binary_hash_invlists fields:
488+
push_val<size_t>(buf, size_t(1)); // sz = 1 (non-zero)
489+
push_val<int>(buf, 0); // il_nbit = 0 (invalid when sz > 0)
490+
491+
expect_binary_read_throws_with(buf, "il_nbit=");
492+
}

0 commit comments

Comments
 (0)