mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
Fix heap-buffer-overflow in ResidualCoarseQuantizer deserialization (T280514309) (#5485)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5485 Fixes T280514309: an attacker-tunable heap-buffer-overflow (write) that fires during `faiss::read_index()` when deserializing a malformed `ResidualCoarseQuantizer` index (`ImRQ` fourcc). Root cause: the FAISS index format encodes the centroid count twice - once as the index header field `ntotal`, and once implicitly as `2^sum(rq.nbits)` (`tot_bits`). On the `ImRQ` read path these two fields are restored independently and never reconciled. `set_beam_factor()` then sizes the `centroid_norms` buffer from `ntotal` (IndexAdditiveQuantizer.cpp:516) while `compute_centroid_norms()` writes `2^tot_bits` floats (AdditiveQuantizer.cpp:386). A crafted index with `ntotal < 2^tot_bits`, `beam_factor <= 0`, and `metric_type == METRIC_L2` overflows the heap allocation. The trained-constructor invariant `FAISS_THROW_IF_NOT(rq.tot_bits <= 63)` and `ntotal == 1 << tot_bits` are bypassed by deserialization, which assigns fields directly onto a default-constructed object. Fix: reconcile the two fields in the `ImRQ` branch of `read_index_up()` before `set_beam_factor()` runs, matching the cross-field consistency checks that sibling index types already perform. We restore the `tot_bits <= 63` bound (also avoiding UB in `(size_t)1 << tot_bits`) and require `ntotal == 2^tot_bits`, which holds by construction for every legitimately-trained ResidualCoarseQuantizer. Reviewed By: mnorris11 Differential Revision: D113832696 fbshipit-source-id: 4bc2f9f5451ba6e6b95cdd33abbe8bafba295b6f
This commit is contained in:
committed by
meta-codesync[bot]
parent
aff8589c9e
commit
a3e8c67c60
@@ -1933,6 +1933,16 @@ std::unique_ptr<Index> read_index_up(IOReader* f, int io_flags) {
|
||||
idxr->ntotal,
|
||||
idxr->rq.M);
|
||||
}
|
||||
FAISS_THROW_IF_NOT_MSG(
|
||||
idxr->rq.tot_bits <= 63,
|
||||
"ResidualCoarseQuantizer tot_bits too large (max 63)");
|
||||
FAISS_THROW_IF_NOT_FMT(
|
||||
static_cast<size_t>(idxr->ntotal) ==
|
||||
(((size_t)1) << idxr->rq.tot_bits),
|
||||
"ResidualCoarseQuantizer ntotal %" PRId64
|
||||
" inconsistent with 2^tot_bits (tot_bits=%zu)",
|
||||
idxr->ntotal,
|
||||
idxr->rq.tot_bits);
|
||||
idxr->set_beam_factor(idxr->beam_factor);
|
||||
idx = std::move(idxr);
|
||||
} else if (
|
||||
|
||||
@@ -2821,8 +2821,8 @@ TEST(ReadIndexDeserialize, AdditiveQuantizerMZero) {
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test: ResidualCoarseQuantizer with ntotal=0 deserializes successfully
|
||||
// (empty index is valid).
|
||||
// Test: ResidualCoarseQuantizer with ntotal inconsistent with its codebooks
|
||||
// is rejected before centroid norms are computed.
|
||||
// -----------------------------------------------------------------------
|
||||
TEST(ReadIndexDeserialize, ResidualCoarseQuantizerNtotalZero) {
|
||||
std::vector<uint8_t> buf;
|
||||
@@ -2831,10 +2831,7 @@ TEST(ReadIndexDeserialize, ResidualCoarseQuantizerNtotalZero) {
|
||||
push_residual_quantizer(buf, /*d=*/4, /*M=*/2, /*nbits=*/{4, 4});
|
||||
push_val<float>(buf, -1.0f); // beam_factor = -1 (skip tables)
|
||||
|
||||
VectorIOReader reader;
|
||||
reader.data = buf;
|
||||
auto idx = read_index_up(&reader);
|
||||
EXPECT_EQ(idx->ntotal, 0);
|
||||
expect_read_throws_with(buf, "inconsistent with 2^tot_bits");
|
||||
}
|
||||
|
||||
// ---- IndexBinaryIVF runtime safety checks ----
|
||||
|
||||
Reference in New Issue
Block a user