Skip to content

Commit 9ea026c

Browse files
scsiguymeta-codesync[bot]
authored andcommitted
Validate codes vector size in BlockInvertedLists deserialization (#4920)
Summary: Pull Request resolved: #4920 After READVECTOR reads ids[i] and codes[i] in BlockInvertedListsIOHook::read(), validate that codes[i].size() matches the expected value computed from ids[i].size(), n_per_block, and block_size. Uses mul_no_overflow to detect integer overflow in the n_block * block_size computation, preventing crafted inputs from bypassing the size check via wraparound. Without this check, a maliciously crafted serialized BlockInvertedLists could have inconsistent codes and ids vectors, leading to out-of-bounds memory access during search or add operations. Reviewed By: mnorris11 Differential Revision: D96346307 fbshipit-source-id: 43156fa287fa004cc9d178fc558d168b7b3848dc
1 parent 9962fbe commit 9ea026c

2 files changed

Lines changed: 102 additions & 0 deletions

File tree

faiss/invlists/BlockInvertedLists.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,21 @@ InvertedLists* BlockInvertedListsIOHook::read(IOReader* f, int /* io_flags */)
184184
for (size_t i = 0; i < il->nlist; i++) {
185185
READVECTOR(il->ids[i]);
186186
READVECTOR(il->codes[i]);
187+
size_t n_ids = il->ids[i].size();
188+
size_t n_block = (n_ids + il->n_per_block - 1) / il->n_per_block;
189+
size_t expected_codes_size = mul_no_overflow(
190+
n_block, il->block_size, "BlockInvertedLists codes");
191+
FAISS_THROW_IF_NOT_FMT(
192+
il->codes[i].size() == expected_codes_size,
193+
"BlockInvertedLists list %zd: codes size %zd does not "
194+
"match expected %zd (ids=%zd, n_per_block=%zd, "
195+
"block_size=%zd)",
196+
i,
197+
il->codes[i].size(),
198+
expected_codes_size,
199+
n_ids,
200+
il->n_per_block,
201+
il->block_size);
187202
}
188203

189204
return il.release();

tests/test_read_index_deserialize.cpp

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,3 +721,90 @@ TEST(ReadIndexDeserialize, BlockInvertedListsBlockSizeZero) {
721721

722722
expect_invlists_read_throws_with(buf, "block_size");
723723
}
724+
725+
// -----------------------------------------------------------------------
726+
// Test: BlockInvertedLists with codes vector size inconsistent with ids
727+
// size, n_per_block, and block_size. Without the check, a search or
728+
// add operation would read/write past the end of the codes buffer.
729+
// -----------------------------------------------------------------------
730+
TEST(ReadIndexDeserialize, BlockInvertedListsCodesSizeMismatch) {
731+
const size_t n_per_block = 32;
732+
const size_t block_size = 128;
733+
734+
std::vector<uint8_t> buf;
735+
push_fourcc(buf, "ilbl");
736+
push_val<size_t>(buf, 1); // nlist = 1
737+
push_val<size_t>(buf, 32); // code_size
738+
push_val<size_t>(buf, n_per_block); // n_per_block
739+
push_val<size_t>(buf, block_size); // block_size
740+
741+
// ids: 10 entries → ceil(10/32) = 1 block → expected codes = 128 bytes
742+
std::vector<int64_t> ids(10, 0);
743+
push_vector<int64_t>(buf, ids);
744+
// codes: 64 bytes (wrong, should be 128)
745+
std::vector<uint8_t> codes(64, 0);
746+
push_vector<uint8_t>(buf, codes);
747+
748+
expect_invlists_read_throws_with(buf, "codes size");
749+
}
750+
751+
// -----------------------------------------------------------------------
752+
// Test: BlockInvertedLists with n_block * block_size overflow. Without
753+
// the mul_no_overflow check, the multiplication wraps around and the
754+
// size comparison passes spuriously.
755+
// -----------------------------------------------------------------------
756+
TEST(ReadIndexDeserialize, BlockInvertedListsCodesOverflow) {
757+
// Choose n_per_block=1 and block_size near SIZE_MAX so that
758+
// n_block * block_size overflows.
759+
const size_t n_per_block = 1;
760+
const size_t block_size = (size_t)-1; // SIZE_MAX
761+
762+
std::vector<uint8_t> buf;
763+
push_fourcc(buf, "ilbl");
764+
push_val<size_t>(buf, 1); // nlist = 1
765+
push_val<size_t>(buf, 32); // code_size
766+
push_val<size_t>(buf, n_per_block); // n_per_block
767+
push_val<size_t>(buf, block_size); // block_size
768+
769+
// ids: 2 entries → n_block = 2 → 2 * SIZE_MAX overflows
770+
std::vector<int64_t> ids(2, 0);
771+
push_vector<int64_t>(buf, ids);
772+
// codes: any size, doesn't matter — overflow should be caught first
773+
std::vector<uint8_t> codes(0);
774+
push_vector<uint8_t>(buf, codes);
775+
776+
expect_invlists_read_throws_with(buf, "overflow");
777+
}
778+
779+
// -----------------------------------------------------------------------
780+
// Test: BlockInvertedLists with valid ids and codes passes validation.
781+
// -----------------------------------------------------------------------
782+
TEST(ReadIndexDeserialize, BlockInvertedListsValidCodesSize) {
783+
const size_t n_per_block = 32;
784+
const size_t block_size = 128;
785+
const size_t nlist = 2;
786+
787+
std::vector<uint8_t> buf;
788+
push_fourcc(buf, "ilbl");
789+
push_val<size_t>(buf, nlist); // nlist
790+
push_val<size_t>(buf, 32); // code_size
791+
push_val<size_t>(buf, n_per_block); // n_per_block
792+
push_val<size_t>(buf, block_size); // block_size
793+
794+
// List 0: 10 ids → ceil(10/32)=1 block → 128 bytes
795+
std::vector<int64_t> ids0(10, 0);
796+
push_vector<int64_t>(buf, ids0);
797+
std::vector<uint8_t> codes0(128, 0);
798+
push_vector<uint8_t>(buf, codes0);
799+
800+
// List 1: 0 ids → 0 blocks → 0 bytes
801+
std::vector<int64_t> ids1;
802+
push_vector<int64_t>(buf, ids1);
803+
std::vector<uint8_t> codes1;
804+
push_vector<uint8_t>(buf, codes1);
805+
806+
VectorIOReader reader;
807+
reader.data = buf;
808+
auto il = read_InvertedLists_up(&reader);
809+
EXPECT_NE(il, nullptr);
810+
}

0 commit comments

Comments
 (0)