mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
Reject nbits==0 ProductQuantizer on deserialization (T279464247) (#5483)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5483 Agentic fuzzing (faiss_rmdt_post_read_reconstruct_fuzzer) found a null-pointer read in PQDecoderGeneric::decode(): a serialized index whose ProductQuantizer declares nbits == 0 derives code_size == 0 (fbcode/faiss/impl/ProductQuantizer.cpp:69), which makes the codes-size consistency check in the deserializer trivially satisfied by an empty codes vector for any ntotal ("0 == ntotal * 0"). The first reconstruct() call then hands codes.data() == nullptr down to PQDecoderGeneric::decode(), which dereferences it unconditionally (reg = *code). D112368333 patched this for the IxPq (IndexPQ) branch specifically by adding a `code_size > 0 || ntotal == 0` guard local to that branch. That missed every other deserialization path that shares the same read_ProductQuantizer() entry point and the same nbits==0 degenerate case -- in particular Index2Layer ("Ix2L"), whose pq.code_size feeds code_size_2 via validate_code_size_match(), which only checks equality and not positivity. The finding kept regressing (crashbot re-flagged it on 2026-07-24, 07-27, 07-28) because the crash still reproduces, now via Index2Layer::sa_decode -> ProductQuantizer::decode -> PQDecoderGeneric::decode. Fix: move the validation to the shared deserializer entry point, read_ProductQuantizer() (fbcode/faiss/impl/index_read.cpp), by tightening the existing nbits upper-bound check into a range check requiring nbits >= 1. This is "Option 1" from the original AI Security Assessment, which the assessment itself noted is the correct place for the check: nbits == 0 is a legitimate in-memory state for a default-constructed, untrained PQ (ProductQuantizer() : ProductQuantizer(0, 1, 0)), so the check can't live in set_derived_values(); it belongs at deserialization, where nbits == 0 always means a corrupt/malicious index. This closes the gap for every current and future caller of read_ProductQuantizer (IxPq, IVFPQ, IVFPQR, Index2Layer, IMI, PQFastScan, ...), not just IndexPQ. Reviewed By: mnorris11 Differential Revision: D114027516 fbshipit-source-id: 571ededc9bd055b2fd220f7d2431e01c9ba7fcbd
This commit is contained in:
committed by
meta-codesync[bot]
parent
88b6c47cf5
commit
5987f523f4
@@ -737,7 +737,9 @@ void read_ProductQuantizer(ProductQuantizer* pq, IOReader* f) {
|
||||
FAISS_THROW_IF_NOT_FMT(
|
||||
pq->M > 0, "invalid ProductQuantizer M=%zd (must be > 0)", pq->M);
|
||||
FAISS_THROW_IF_NOT_FMT(
|
||||
pq->nbits <= 24, "invalid ProductQuantizer nbits=%zd", pq->nbits);
|
||||
pq->nbits >= 1 && pq->nbits <= 24,
|
||||
"invalid ProductQuantizer nbits=%zd (must be in [1, 24])",
|
||||
pq->nbits);
|
||||
{
|
||||
size_t ksub = size_t{1} << pq->nbits;
|
||||
size_t n = mul_no_overflow(pq->d, ksub, "PQ centroids");
|
||||
|
||||
Reference in New Issue
Block a user