Skip to content

Validate code_size against compute_code_size in RaBitQ flat deserialization - #5432

Open
AlexMelanFromRingo wants to merge 1 commit into
facebookresearch:mainfrom
AlexMelanFromRingo:validate-rabitq-code-size-field
Open

Validate code_size against compute_code_size in RaBitQ flat deserialization#5432
AlexMelanFromRingo wants to merge 1 commit into
facebookresearch:mainfrom
AlexMelanFromRingo:validate-rabitq-code-size-field

Conversation

@AlexMelanFromRingo

Copy link
Copy Markdown

Summary

The flat RaBitQ readers (fourcc("Ixrq") / fourcc("Ixrr") in read_index, faiss/impl/index_read.cpp) take rabitq.code_size straight from the file and use it as the per-vector stride, without checking it against compute_code_size(d, nb_bits). The IVF RaBitQ readers (Iwrq/Iwrr) already recompute code_size after reading (ivrq->rabitq.code_size = ivrq->rabitq.compute_code_size(...)), so only the flat readers trust the stored value.

RaBitQuantizer::decode_core strides by code_size but locates the per-code factor block at offset (d + 7) / 8, derived from d rather than code_size. A file with a too-small code_size therefore makes search() / sa_decode() read the factor block past the end of the codes buffer.

This is independent of the codes.size() == ntotal * code_size buffer-length check (added for these arms in #5293): a codes buffer sized to ntotal * (forged code_size) satisfies that check yet still overruns, because the factor read within the final code slot extends past the buffer end.

Reproduce (heap OOB under AddressSanitizer, public API only)

A self-contained program that builds a crafted Ixrq payload (d=8, forged code_size=1 vs real 9, codes buffer = ntotal bytes so the codes.size() check passes), then read_index + sa_decode:

read_index SUCCEEDED. loaded code_size=1, codes.size()=64
calling sa_decode ...
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 4 at 0x... 0 bytes after 64-byte region
  #0 faiss::RaBitQuantizer::decode_core(...)      faiss/impl/RaBitQuantizer.cpp:219
  #1 faiss::IndexRaBitQ::sa_decode(...)           faiss/IndexRaBitQ.cpp:61
0x...120 is located 0 bytes after 64-byte region
  allocated by read_index_up                      faiss/impl/index_read.cpp (codes buffer)

With this patch, read_index rejects the file up front:

FaissException: IndexRaBitQ code_size mismatch: stored 1 vs derived 9
  at validate_code_size_match  faiss/impl/index_read.cpp

Fix

Validate the stored code_size against compute_code_size(d, nb_bits) in both flat arms, using the existing validate_code_size_match helper (this is the scalar stored-vs-derived case that helper is for).

Also add RaBitQCodeSizeFieldMismatch_Ixrq / _Ixrr regression tests, and fix the push_rabitq / push_rabitq_multibit test helpers to emit a valid code_size — they previously wrote a placeholder that only passed because the reader did not validate it.

Validated locally: new tests pass with the change and fail without it; the full ReadIndexDeserialize suite passes; and the ASAN reproduction above is rejected instead of overrunning.

Related to #5293 (same flat readers, complementary buffer-length check). The two are independent: #5293 bounds codes.size(); this PR fixes code_size itself, which #5293 alone does not cover.

🤖 Generated with Claude Code

…zation

The flat RaBitQ readers (fourcc Ixrq/Ixrr) take rabitq.code_size straight
from the file and use it as the per-vector stride, without checking it
against compute_code_size(d, nb_bits). The IVF RaBitQ readers (Iwrq/Iwrr)
already recompute code_size after reading, so only the flat readers trust
the stored value.

decode_core() strides by code_size but locates the per-code factor block
at offset (d + 7) / 8, derived from d rather than code_size. A file with a
too-small code_size therefore makes search()/sa_decode() read the factor
block past the end of the codes buffer. This is independent of the
codes.size() == ntotal * code_size buffer-length check: a buffer sized to
ntotal * (forged code_size) satisfies that check yet still overruns,
because the factor read within the final code slot extends past it.

Reproduced under AddressSanitizer via the public read_index + sa_decode
API (d=8, forged code_size=1 vs real 9, codes buffer = ntotal bytes):
heap-buffer-overflow READ in RaBitQuantizer::decode_core, on the codes
buffer allocated in read_index_up. With this change read_index instead
rejects the file with "IndexRaBitQ code_size mismatch: stored 1 vs
derived 9".

Validate the stored code_size against compute_code_size in both flat arms
using the existing validate_code_size_match helper. Add
RaBitQCodeSizeFieldMismatch_Ixrq / _Ixrr regression tests, and fix the
push_rabitq / push_rabitq_multibit test helpers to emit a valid code_size
(they previously wrote a placeholder that only passed because the reader
did not validate it).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed label Jul 16, 2026
@meta-codesync

meta-codesync Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@bshethmeta has imported this pull request. If you are a Meta employee, you can view this in D112827997.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant