From fe46c3c804296cc04238c2e17b02c671887791a8 Mon Sep 17 00:00:00 2001 From: Justin Gibbs Date: Thu, 4 Jun 2026 18:16:44 -0700 Subject: [PATCH] Validate bool fields during deserialization (#5279) Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5279 READ1(bool) reads a single byte directly from the serialized stream into a bool's storage. The set of valid byte values for a bool is implementation-defined - on every C++ ABI any byte outside the implementation's canonical encoding is undefined behavior, and UBSan's invalid-bool-load check aborts on it. FAISS itself only ever writes the canonical encoding via WRITE1(bool), so well-formed indices roundtrip cleanly. Corrupt or attacker-controlled input that places a non-canonical byte at a bool-field offset, however, can introduce UB-inducing data instead of being rejected as a deserialization error. Add a READ1_BOOL helper to faiss/impl/io_macros.h that reads one byte, assigns it to the destination bool via the language-defined conversion (b != 0) - which is well-defined for any input byte - and then compares the resulting bool's storage byte back against the byte we read. The roundtrip succeeds iff the input byte was already the platform's canonical bool encoding. This stays ABI-portable: no assumption that 0 and 1 are the only valid encodings is baked into the source. A static_assert(sizeof(bool) == 1) makes the existing implicit FAISS assumption (WRITE1/READ1 of bool transfers exactly one byte) explicit at the one call site that depends on it for the byte compare. Apply READ1_BOOL to every READ1 call in faiss/impl/index_read.cpp whose destination is a bool - covering the local SVS variables (initialized, trained, has_vt) and bool fields across IVF, PQ, HNSW, NSG, NNDescent, SVS, additive-quantizer, lattice, scalar-quantizer, refine, residual, and binary index types. Reviewed By: mnorris11 Differential Revision: D107454749 fbshipit-source-id: bc550083a83e9bd5fe09a58a9b698e9380ef9cc1 --- faiss/impl/index_read.cpp | 78 +++++++++++++-------------- faiss/impl/io_macros.h | 25 +++++++++ tests/test_read_index_deserialize.cpp | 26 +++++++++ 3 files changed, 90 insertions(+), 39 deletions(-) diff --git a/faiss/impl/index_read.cpp b/faiss/impl/index_read.cpp index dfdad0145..8d1ae520f 100644 --- a/faiss/impl/index_read.cpp +++ b/faiss/impl/index_read.cpp @@ -288,7 +288,7 @@ static void read_index_header(Index& idx, IOReader* f) { idx_t dummy; READ1(dummy); READ1(dummy); - READ1(idx.is_trained); + READ1_BOOL(idx.is_trained); int metric_type_int; READ1(metric_type_int); idx.metric_type = metric_type_from_int(metric_type_int); @@ -316,7 +316,7 @@ std::unique_ptr read_VectorTransform_up(IOReader* f) { if (h == fourcc("Pcam")) { READ1(pca->epsilon); } - READ1(pca->random_rotation); + READ1_BOOL(pca->random_rotation); if (h != fourcc("PCAm")) { READ1(pca->balanced_bins); } @@ -332,7 +332,7 @@ std::unique_ptr read_VectorTransform_up(IOReader* f) { } else if (h == fourcc("LTra")) { lt = std::make_unique(); } - READ1(lt->have_bias); + READ1_BOOL(lt->have_bias); READVECTOR(lt->A); READVECTOR(lt->b); FAISS_THROW_IF_NOT( @@ -356,7 +356,7 @@ std::unique_ptr read_VectorTransform_up(IOReader* f) { auto itqt = std::make_unique(); READVECTOR(itqt->mean); - READ1(itqt->do_pca); + READ1_BOOL(itqt->do_pca); { // Read, dereference, discard. auto sub_vt = read_VectorTransform_up(f); @@ -385,7 +385,7 @@ std::unique_ptr read_VectorTransform_up(IOReader* f) { } READ1(vt->d_in); READ1(vt->d_out); - READ1(vt->is_trained); + READ1_BOOL(vt->is_trained); FAISS_THROW_IF_NOT_FMT( vt->d_in >= 0, "invalid VectorTransform d_in=%d (must be >= 0)", @@ -771,7 +771,7 @@ static void read_ResidualQuantizer_old(ResidualQuantizer& rq, IOReader* f) { "ResidualQuantizer nbits size %zd != M %zd", rq.nbits.size(), rq.M); - READ1(rq.is_trained); + READ1_BOOL(rq.is_trained); READ1(rq.train_type); READ1(rq.max_beam_size); READVECTOR(rq.codebooks); @@ -789,7 +789,7 @@ static void read_AdditiveQuantizer(AdditiveQuantizer& aq, IOReader* f) { FAISS_THROW_IF_NOT_FMT( aq.M > 0, "invalid AdditiveQuantizer M %zd, must be > 0", aq.M); READVECTOR(aq.nbits); - READ1(aq.is_trained); + READ1_BOOL(aq.is_trained); READVECTOR(aq.codebooks); FAISS_THROW_IF_NOT_FMT( aq.nbits.size() == aq.M, @@ -1279,7 +1279,7 @@ static void read_NSG(NSG& nsg, IOReader* f) { READ1(nsg.C); READ1(nsg.search_L); READ1(nsg.enterpoint); - READ1(nsg.is_built); + READ1_BOOL(nsg.is_built); FAISS_THROW_IF_NOT_FMT( nsg.ntotal >= 0, "invalid NSG ntotal %d", nsg.ntotal); @@ -1331,7 +1331,7 @@ static void read_NNDescent(NNDescent& nnd, IOReader* f) { READ1(nnd.iter); READ1(nnd.search_L); READ1(nnd.random_seed); - READ1(nnd.has_built); + READ1_BOOL(nnd.has_built); FAISS_THROW_IF_NOT_FMT( nnd.ntotal >= 0, "invalid NNDescent ntotal %d", nnd.ntotal); @@ -1470,7 +1470,7 @@ static std::unique_ptr read_ivfpq( std::vector> ids; read_ivf_header(ivpq.get(), f, legacy ? &ids : nullptr); - READ1(ivpq->by_residual); + READ1_BOOL(ivpq->by_residual); READ1(ivpq->code_size); read_ProductQuantizer(&ivpq->pq, f); @@ -1539,7 +1539,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { d, n_levels, batch_size); } READ1(idxp->ntotal); - READ1(idxp->is_trained); + READ1_BOOL(idxp->is_trained); READVECTOR(idxp->codes); READVECTOR(idxp->cum_sums); idxp->verbose = false; @@ -1564,8 +1564,8 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { auto idxl = std::make_unique(); read_index_header(*idxl, f); READ1(idxl->nbits); - READ1(idxl->rotate_data); - READ1(idxl->train_thresholds); + READ1_BOOL(idxl->rotate_data); + READ1_BOOL(idxl->train_thresholds); READVECTOR(idxl->thresholds); int code_size_i; READ1(code_size_i); @@ -1611,7 +1611,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { idxp->codes.size() == idxp->ntotal * idxp->code_size); if (h == fourcc("IxPo") || h == fourcc("IxPq")) { READ1(idxp->search_type); - READ1(idxp->encode_signs); + READ1_BOOL(idxp->encode_signs); READ1(idxp->polysemous_ht); } // Old versions of PQ all had metric_type set to INNER_PRODUCT @@ -1774,7 +1774,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READ1(idxaqfs->ntotal2); READ1(idxaqfs->M2); - READ1(idxaqfs->rescale_norm); + READ1_BOOL(idxaqfs->rescale_norm); READ1(idxaqfs->norm_scale); READ1(idxaqfs->max_train_points); @@ -1824,7 +1824,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { validate_aq_dimension_match( *ivaqfs->aq, ivaqfs->d, "IndexIVFAdditiveQuantizerFastScan"); - READ1(ivaqfs->by_residual); + READ1_BOOL(ivaqfs->by_residual); READ1(ivaqfs->implem); READ1(ivaqfs->bbs); READ1(ivaqfs->qbs); @@ -1837,7 +1837,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READ1(ivaqfs->qbs2); READ1(ivaqfs->M2); - READ1(ivaqfs->rescale_norm); + READ1_BOOL(ivaqfs->rescale_norm); READ1(ivaqfs->norm_scale); READ1(ivaqfs->max_train_points); @@ -2007,7 +2007,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { if (h == fourcc("IwSQ")) { ivsc->by_residual = true; } else { - READ1(ivsc->by_residual); + READ1_BOOL(ivsc->by_residual); } read_InvertedLists(*ivsc, f, io_flags); idx = std::move(ivsc); @@ -2046,7 +2046,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { iva->code_size, iva->aq->code_size, "IndexIVFAdditiveQuantizer"); - READ1(iva->by_residual); + READ1_BOOL(iva->by_residual); READ1(iva->use_precomputed_table); read_InvertedLists(*iva, f, io_flags); idx = std::move(iva); @@ -2073,7 +2073,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { read_index_header(*indep, f); indep->quantizer = read_index(f, io_flags); bool has_vt; - READ1(has_vt); + READ1_BOOL(has_vt); if (has_vt) { indep->vt = read_VectorTransform(f); } @@ -2269,11 +2269,11 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READVECTOR(idx_panorama->cum_sums); } if (h == fourcc("IHNc") || h == fourcc("IHc2")) { - READ1(idxhnsw->keep_max_size_level0); + READ1_BOOL(idxhnsw->keep_max_size_level0); auto idx_hnsw_cagra = dynamic_cast(idxhnsw.get()); FAISS_THROW_IF_NOT_MSG( idx_hnsw_cagra, "dynamic_cast to IndexHNSWCagra failed"); - READ1(idx_hnsw_cagra->base_level_only); + READ1_BOOL(idx_hnsw_cagra->base_level_only); READ1(idx_hnsw_cagra->num_base_level_search_entrypoints); if (h == fourcc("IHc2")) { READ1(idx_hnsw_cagra->numeric_type_); @@ -2430,7 +2430,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { } else if (h == fourcc("IwPf")) { auto ivpq = std::make_unique(); read_ivf_header(ivpq.get(), f); - READ1(ivpq->by_residual); + READ1_BOOL(ivpq->by_residual); READ1(ivpq->code_size); READ1(ivpq->bbs); READ1(ivpq->M2); @@ -2570,7 +2570,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { read_ivf_header(ivrq.get(), f); read_RaBitQuantizer(ivrq->rabitq, f, ivrq->d, false); READ1(ivrq->code_size); - READ1(ivrq->by_residual); + READ1_BOOL(ivrq->by_residual); READ1(ivrq->qb); // qb=0: Not quantized - direct distance computation on given float32s. // qb>0 && qb<=8: Scalar-quantized with qb bits of precision. @@ -2593,7 +2593,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { read_RaBitQuantizer( ivrq->rabitq, f, ivrq->d, true); // Reads nb_bits from file READ1(ivrq->code_size); - READ1(ivrq->by_residual); + READ1_BOOL(ivrq->by_residual); READ1(ivrq->qb); // qb=0: Not quantized - direct distance computation on given float32s. // qb>0 && qb<=8: Scalar-quantized with qb bits of precision. @@ -2630,10 +2630,10 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READ1(svs->construction_window_size); READ1(svs->max_candidate_pool_size); READ1(svs->prune_to); - READ1(svs->use_full_search_history); + READ1_BOOL(svs->use_full_search_history); svs->storage_kind = read_svs_storage_kind(f); - READ1(svs->is_static); + READ1_BOOL(svs->is_static); if (h == fourcc("ISVL")) { auto* leanvec = dynamic_cast(svs.get()); @@ -2643,7 +2643,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { } bool initialized; - READ1(initialized); + READ1_BOOL(initialized); if (initialized) { faiss::svs_io::ReaderStreambuf rbuf( f, get_deserialization_vector_byte_limit()); @@ -2652,7 +2652,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { } if (h == fourcc("ISVL")) { bool trained; - READ1(trained); + READ1_BOOL(trained); if (trained) { faiss::svs_io::ReaderStreambuf rbuf( f, get_deserialization_vector_byte_limit()); @@ -2675,7 +2675,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { read_index_header(*svs, f); bool initialized; - READ1(initialized); + READ1_BOOL(initialized); if (initialized) { faiss::svs_io::ReaderStreambuf rbuf( f, get_deserialization_vector_byte_limit()); @@ -2698,7 +2698,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READ1(svs_ivf->num_centroids); READ1(svs_ivf->minibatch_size); READ1(svs_ivf->num_iterations); - READ1(svs_ivf->is_hierarchical); + READ1_BOOL(svs_ivf->is_hierarchical); READ1(svs_ivf->training_fraction); READ1(svs_ivf->hierarchical_level1_clusters); READ1(svs_ivf->seed); @@ -2707,7 +2707,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { READ1(svs_ivf->num_threads); READ1(svs_ivf->intra_query_threads); svs_ivf->storage_kind = read_svs_storage_kind(f); - READ1(svs_ivf->is_static); + READ1_BOOL(svs_ivf->is_static); if (h == fourcc("ISIL")) { auto* leanvec = dynamic_cast(svs_ivf.get()); FAISS_THROW_IF_NOT_MSG( @@ -2716,7 +2716,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { } bool initialized; - READ1(initialized); + READ1_BOOL(initialized); if (initialized) { faiss::svs_io::ReaderStreambuf rbuf(f); std::istream is(&rbuf); @@ -2724,7 +2724,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { } if (h == fourcc("ISIL")) { bool trained; - READ1(trained); + READ1_BOOL(trained); if (trained) { faiss::svs_io::ReaderStreambuf rbuf(f); std::istream is(&rbuf); @@ -2746,7 +2746,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { auto ivrqfs = std::make_unique(); read_ivf_header(ivrqfs.get(), f); read_RaBitQuantizer(ivrqfs->rabitq, f, ivrqfs->d); - READ1(ivrqfs->by_residual); + READ1_BOOL(ivrqfs->by_residual); READ1(ivrqfs->code_size); READ1(ivrqfs->bbs); READ1(ivrqfs->qbs2); @@ -2757,7 +2757,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { ivrqfs->qb > 0 && ivrqfs->qb <= 8, "invalid RaBitQ qb=%d (must be in [1, 8])", ivrqfs->qb); - READ1(ivrqfs->centered); + READ1_BOOL(ivrqfs->centered); std::vector legacy_flat_storage; if (is_legacy) { @@ -2884,7 +2884,7 @@ static void read_index_binary_header(IndexBinary& idx, IOReader* f) { READ1(idx.d); READ1(idx.code_size); READ1(idx.ntotal); - READ1(idx.is_trained); + READ1_BOOL(idx.is_trained); int metric_type_int; READ1(metric_type_int); idx.metric_type = metric_type_from_int(metric_type_int); @@ -3070,8 +3070,8 @@ std::unique_ptr read_index_binary_up(IOReader* f, int io_flags) { } else if (h == fourcc("IBHc")) { auto idxhnsw = std::make_unique(); read_index_binary_header(*idxhnsw, f); - READ1(idxhnsw->keep_max_size_level0); - READ1(idxhnsw->base_level_only); + READ1_BOOL(idxhnsw->keep_max_size_level0); + READ1_BOOL(idxhnsw->base_level_only); READ1(idxhnsw->num_base_level_search_entrypoints); read_HNSW(idxhnsw->hnsw, f); idxhnsw->hnsw.is_panorama = false; diff --git a/faiss/impl/io_macros.h b/faiss/impl/io_macros.h index 5e0633606..c8ed20471 100644 --- a/faiss/impl/io_macros.h +++ b/faiss/impl/io_macros.h @@ -35,6 +35,31 @@ size_t get_deserialization_vector_byte_limit(); #define READ1(x) READANDCHECK(&(x), 1) +// Reads a single byte into a bool, rejecting any byte that is not the +// canonical encoding for the platform's bool representation. Reading a +// non-canonical byte directly into a bool is undefined behavior and +// trips UBSan's invalid-bool-load check. To stay ABI-portable, we +// assign via the language-defined conversion (b != 0) and then compare +// the resulting bool's storage byte back against the byte we read - the +// roundtrip succeeds iff the input byte was already canonical on this +// platform. FAISS only ever writes the canonical encoding via +// WRITE1(bool), so well-formed indices roundtrip cleanly; corrupt or +// attacker-controlled input that places a non-canonical byte at a bool +// offset is rejected as a FaissException. +#define READ1_BOOL(x) \ + { \ + static_assert( \ + sizeof(x) == 1, "READ1_BOOL: destination must be 1 byte"); \ + uint8_t b; \ + READANDCHECK(&b, 1); \ + (x) = (b != 0); \ + FAISS_THROW_IF_NOT_FMT( \ + *reinterpret_cast(&(x)) == b, \ + "invalid bool encoding 0x%02x for %s", \ + b, \ + #x); \ + } + #define READ1_DUMMY(x_type) \ { \ x_type x = {}; \ diff --git a/tests/test_read_index_deserialize.cpp b/tests/test_read_index_deserialize.cpp index eb34d6e0d..6c49f725d 100644 --- a/tests/test_read_index_deserialize.cpp +++ b/tests/test_read_index_deserialize.cpp @@ -151,6 +151,32 @@ TEST(ReadIndexDeserialize, PQWithMZeroDivideByZero) { expect_read_throws(buf); } +// ----------------------------------------------------------------------- +// Test: a non-{0,1} byte at a bool field position must be rejected as +// FaissException rather than silently producing an invalid bool value. +// Reading a non-canonical byte directly into a bool is UB and trips +// UBSan's invalid-bool-load check; the READ1_BOOL helper routes every +// bool READ1 through a value check and throws on anything outside {0,1}. +// is_trained, read inside read_index_header, exercises the helper from +// the most common deserialization path. +// ----------------------------------------------------------------------- +TEST(ReadIndexDeserialize, IndexHeaderInvalidBoolEncodingIsTrained) { + // Minimal IndexFlatL2 ("IxF2") with ntotal=0 - read_index_header is + // the first thing the deserializer consumes for this fourcc. + std::vector buf; + push_fourcc(buf, "IxF2"); + push_index_header(buf, /*d=*/4, /*ntotal=*/0); + + // is_trained sits at offset 4 (fourcc) + 4 (d) + 8 (ntotal) + + // 8 (dummy) + 8 (dummy) = 32. push_index_header writes 0x01 there; + // overwrite with a non-canonical bool byte. + constexpr size_t is_trained_offset = 32; + ASSERT_EQ(buf[is_trained_offset], uint8_t{1}); + buf[is_trained_offset] = 0x02; + + expect_read_throws_with(buf, "invalid bool encoding"); +} + // ----------------------------------------------------------------------- // Test: AdditiveQuantizer with nbits.size() != M causes out-of-bounds // access on the nbits vector in set_derived_values(). The fix validates