[8.2] [MOD-11650] Fix Out-of-Bounds Write in Vector Preprocessing (#794)

[MOD-11650] Fix Out-of-Bounds Write in Vector Preprocessing (#784)

* rename dataSize->getStoredDataSize

* missing rename

* format

* add input blob size to AbstractIndexInitParams

in VecSimIndexAbstract move data members to private when possible

DONT FIX LEAK YET

Factories:
move NewAbstractInitParams to a general location (new file factory_utils)
replace it in all factories

* try sanitizer

* run sanitizer

* runanyway

* rervt task unit test
use sanitizer in pull request

* fix leak in input bob size

* fix int8/uint8 elementSizeEstimation tests

This caused a buffer overflow because setup<TieredIndexParams> didn't set the data type to INT8, creating a float32 index instead.
DataBlock::addElement() tries to copy dim * sizeof(float) bytes, but the allocated buffer is only dim * sizeof(int8) bytes, causing a read overflow.

* run codecov with sanitizer (also intek)

* some fixes and assertion

* fix uint8

* fix again

* fix possible warning abour divison by zero by checking quantBits with constexpr

* TO REVERT!

fix leaks that will be moved to a separate PR
it was failing only with codecov becuase only there we use FP64_TESTS=1

Prevent template deduction errors in GenerateAndAddVector by making data_t parameter non-deducible

Used std::type_identity<data_t>::type for the value parameter to force explicit template specification (e.g., GenerateAndAddVector<double>()) instead of allowing compiler to incorrectly deduce int from literal values, which caused buffer overflows when index expected different data types.

* Revert "TO REVERT!"

This reverts commit af844ec.

* rever ci changes

* calculate EstimateElementSize accroding to the stored vector size
add tests

* revert unrelated change in cmake.san

* add batch itertor blob correctness to int8 tests

fix getQueryBlob in tiered
add getHNSWIterator to tiered batch itertor if its BUILD_TESTS

* apply suggesting

(cherry picked from commit 9fb223a)

Author: meiravgri

cmake/san.cmake

@@ -20,4 +20,4 @@ if (USE_ASAN OR USE_MSAN)
 		set(LLVM_CXX_FLAGS "-stdlib=libc++ -I${MSAN_PREFIX}/include -I${MSAN_PREFIX}/include/c++/v1")
 		set(LLVM_LD_FLAGS "-stdlib=libc++ -Wl,-rpath=${MSAN_PREFIX}/lib -L${MSAN_PREFIX}/lib -lc++abi")
 	endif()
-endif()
+endif()

src/VecSim/algorithms/brute_force/brute_force_multi.h

@@ -63,8 +63,8 @@ class BruteForceIndex_Multi : public BruteForceIndex<DataType, DistType> {
             const char *data = reinterpret_cast<const char *>(this->getDataByInternalId(id));
 
             // Create a vector with the full data (including any metadata like norms)
-            std::vector<char> vec(this->getDataSize());
-            memcpy(vec.data(), data, this->getDataSize());
+            std::vector<char> vec(this->getStoredDataSize());
+            memcpy(vec.data(), data, this->getStoredDataSize());
             vectors_output.push_back(std::move(vec));
         }
 

src/VecSim/algorithms/brute_force/brute_force_single.h

@@ -63,8 +63,8 @@ class BruteForceIndex_Single : public BruteForceIndex<DataType, DistType> {
         const char *data = reinterpret_cast<const char *>(this->getDataByInternalId(id));
 
         // Create a vector with the full data (including any metadata like norms)
-        std::vector<char> vec(this->getDataSize());
-        memcpy(vec.data(), data, this->getDataSize());
+        std::vector<char> vec(this->getStoredDataSize());
+        memcpy(vec.data(), data, this->getStoredDataSize());
         vectors_output.push_back(std::move(vec));
 
         return vectors_output;

src/VecSim/algorithms/hnsw/hnsw.h

@@ -1182,7 +1182,7 @@ void HNSWIndex<DataType, DistType>::SwapLastIdWithDeletedId(idType element_inter
     memcpy((void *)element, last_element, this->elementGraphDataSize);
 
     auto data = getDataByInternalId(element_internal_id);
-    memcpy((void *)data, last_element_data, this->dataSize);
+    memcpy((void *)data, last_element_data, this->getStoredDataSize());
 
     this->idToMetaData[element_internal_id] = this->idToMetaData[curElementCount];
 

src/VecSim/algorithms/hnsw/hnsw_multi.h

@@ -90,8 +90,8 @@ class HNSWIndex_Multi : public HNSWIndex<DataType, DistType> {
             const char *data = this->getDataByInternalId(id);
 
             // Create a vector with the full data (including any metadata like norms)
-            std::vector<char> vec(this->dataSize);
-            memcpy(vec.data(), data, this->dataSize);
+            std::vector<char> vec(this->getStoredDataSize());
+            memcpy(vec.data(), data, this->getStoredDataSize());
             vectors_output.push_back(std::move(vec));
         }
 

src/VecSim/algorithms/hnsw/hnsw_single.h

@@ -62,8 +62,8 @@ class HNSWIndex_Single : public HNSWIndex<DataType, DistType> {
         const char *data = this->getDataByInternalId(id);
 
         // Create a vector with the full data (including any metadata like norms)
-        std::vector<char> vec(this->dataSize);
-        memcpy(vec.data(), data, this->dataSize);
+        std::vector<char> vec(this->getStoredDataSize());
+        memcpy(vec.data(), data, this->getStoredDataSize());
         vectors_output.push_back(std::move(vec));
 
         return vectors_output;

src/VecSim/algorithms/hnsw/hnsw_tiered.h

@@ -179,11 +179,17 @@ class TieredHNSWIndex : public VecSimTieredIndex<DataType, DistType> {
 
         ~TieredHNSW_BatchIterator();
 
+        const void *getQueryBlob() const override { return flat_iterator->getQueryBlob(); }
+
         VecSimQueryReply *getNextResults(size_t n_res, VecSimQueryReply_Order order) override;
 
         bool isDepleted() override;
 
         void reset() override;
+
+#ifdef BUILD_TESTS
+        VecSimBatchIterator *getHNSWIterator() { return hnsw_iterator; }
+#endif
     };
 
 public:
@@ -542,7 +548,7 @@ void TieredHNSWIndex<DataType, DistType>::executeInsertJob(HNSWInsertJob *job) {
     HNSWIndex<DataType, DistType> *hnsw_index = this->getHNSWIndex();
     // Copy the vector blob from the flat buffer, so we can release the flat lock while we are
     // indexing the vector into HNSW index.
-    size_t data_size = this->frontendIndex->getDataSize();
+    size_t data_size = this->frontendIndex->getStoredDataSize();
     auto blob_copy = this->getAllocator()->allocate_unique(data_size);
     // Assuming the size of the blob stored in the frontend index matches the size of the blob
     // stored in the HNSW index.

src/VecSim/algorithms/svs/svs.h

@@ -182,12 +182,13 @@ class SVSIndex : public VecSimIndexAbstract<svs_details::vecsim_dt<DataType>, fl
             return MemoryUtils::unique_blob{const_cast<void *>(original_data), [](void *) {}};
         }
 
-        const auto data_size = this->getDataSize() * n;
+        const auto data_size = this->getStoredDataSize() * n;
 
         auto processed_blob =
             MemoryUtils::unique_blob{this->allocator->allocate(data_size),
                                      [this](void *ptr) { this->allocator->free_allocation(ptr); }};
         // Assuming original data size equals to processed data size
+        assert(this->getInputBlobSize() == this->getStoredDataSize());
         memcpy(processed_blob.get(), original_data, data_size);
         // Preprocess each vector in place
         for (size_t i = 0; i < n; i++) {

src/VecSim/algorithms/svs/svs_serializer_impl.h

@@ -24,7 +24,7 @@ void SVSIndex<MetricType, DataType, isMulti, QuantBits, ResidualBits, IsLeanVec>
     // Note: this->metric corresponds to MetricType template parameter
     writeBinaryPOD(output, this->dim);
     writeBinaryPOD(output, this->vecType); // DataType template parameter (as VecSimType enum)
-    writeBinaryPOD(output, this->dataSize);
+    writeBinaryPOD(output, this->getStoredDataSize());
     writeBinaryPOD(output, this->metric); // MetricType template parameter (as VecSimMetric enum)
     writeBinaryPOD(output, this->blockSize);
     writeBinaryPOD(output, this->isMulti);
@@ -128,7 +128,7 @@ bool SVSIndex<MetricType, DataType, isMulti, QuantBits, ResidualBits,
 
     compareField(input, this->dim, "dim");
     compareField(input, this->vecType, "vecType");
-    compareField(input, this->dataSize, "dataSize");
+    compareField(input, this->getStoredDataSize(), "dataSize");
     compareField(input, this->metric, "metric");
     compareField(input, this->blockSize, "blockSize");
     compareField(input, this->isMulti, "isMulti");

src/VecSim/batch_iterator.h

@@ -29,7 +29,7 @@ struct VecSimBatchIterator : public VecsimBaseObject {
         : VecsimBaseObject(allocator), query_vector(query_vector), returned_results_count(0),
           timeoutCtx(tctx) {};
 
-    inline const void *getQueryBlob() const { return query_vector; }
+    virtual inline const void *getQueryBlob() const { return query_vector; }
 
     inline void *getTimeoutCtx() const { return timeoutCtx; }