Skip to content

Commit

Permalink
MB-62221: Fix buffer overflow (#29)
Browse files Browse the repository at this point in the history
- The `faiss::VectorIOWriter.data` is a vector of type `uint8_t`. However,
   the code attempts to copy this vector to a malloc'd array of type `unsigned char`.
   This results in platform-dependent code since `uint8_t` is platform-agnostic,
   whereas `unsigned char` is not defined consistently across different platforms
   in the C standard.
- This issue becomes apparent when using `std::copy`, where the effective
   signature is `std::copy(uint8_t*, uint8_t*, unsigned char*)`.
- According to the C++ STL template for `copy`, both the StartPointer and
   DestinationPointer are incremented by 1. The behavior of this pointer arithmetic
   depends on the type of pointer passed. Since `uint8_t*` is platform-agnostic
   and `unsigned char*` is platform-dependent, a buffer overflow could occur,
   leading to undefined behavior for `std::copy()` as per the C++ standard.
- Also fixes a mismatched header file where the signature of
   `faiss_read_index_buf` was not matching the one defined in the source cpp file.
  • Loading branch information
CascadingRadium authored Jun 12, 2024
1 parent 7977457 commit a2f4183
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions c_api/index_io_c_ex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
using faiss::Index;
using faiss::IndexBinary;

int faiss_write_index_buf(const FaissIndex* idx, size_t* size, unsigned char** buf) {
int faiss_write_index_buf(const FaissIndex* idx, size_t* size, uint8_t** buf) {
try {
faiss::VectorIOWriter writer;
faiss::write_index(reinterpret_cast<const Index*>(idx), &writer);
unsigned char* tempBuf = (unsigned char*)malloc((writer.data.size()) * sizeof(uint8_t));
uint8_t* tempBuf = (uint8_t*)malloc((writer.data.size()) * sizeof(uint8_t));
std::copy(writer.data.begin(), writer.data.end(), tempBuf);
*buf = tempBuf;
*size = writer.data.size();
Expand Down
4 changes: 2 additions & 2 deletions c_api/index_io_c_ex.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ extern "C" {

/** Write index to buffer
*/
int faiss_write_index_buf(const FaissIndex* idx, size_t* buf_size, unsigned char** buf);
int faiss_write_index_buf(const FaissIndex* idx, size_t* buf_size, uint8_t** buf);

/** Read index from buffer
*/
int faiss_read_index_buf(const unsigned char* buf, size_t limit, int io_flags,
int faiss_read_index_buf(const uint8_t* buf, size_t limit, int io_flags,
FaissIndex** p_out);

#ifdef __cplusplus
Expand Down

0 comments on commit a2f4183

Please # to comment.