From 57355075e63026949a7f88f9e8d7d6036860c8b9 Mon Sep 17 00:00:00 2001 From: Evgen Byelozorov Date: Sat, 18 Jul 2026 14:30:12 +0200 Subject: [PATCH] fix(hnsw): reject index files whose vector count overflows size_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hnsw_load_impl read n_nodes and dim straight from the file header and sized the vectors allocation as n_nodes * dim * sizeof(float) with no overflow check. A crafted header could make that product wrap size_t, so ray_sys_alloc under-allocated the buffer while the following fread still read the full (large) element count and wrote past the allocation — a heap-overflow write driven by an untrusted index file. Factor the check into ray_hnsw_vec_size_valid(n_nodes, dim) and reject the header before any allocation, mirroring the per-layer neighbor guard. Add a unit test that drives the helper directly (ordinary dims, non-positive dims, an overflowing pair, and the exact size_t boundary). It is tested at the helper rather than through ray_hnsw_load because an overflow-patched header is refused earlier — the huge node-level read fails first — so a full-load test could not distinguish the guard. --- src/store/hnsw.c | 16 ++++++++++++++++ src/store/hnsw.h | 5 +++++ test/test_embedding.c | 27 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/src/store/hnsw.c b/src/store/hnsw.c index fb53b4b42..339119a19 100644 --- a/src/store/hnsw.c +++ b/src/store/hnsw.c @@ -895,6 +895,18 @@ ray_err_t ray_hnsw_save(const ray_hnsw_t* idx, const char* dir) { return RAY_OK; } +/* Whether the vectors block — n_nodes * dim * sizeof(float) — fits in size_t. + * Split out of hnsw_load_impl so the overflow guard can be unit-tested + * directly. n_nodes and dim come from an untrusted on-disk header; if the + * product wraps size_t the allocation under-sizes the buffer that the + * following fread then overruns (a heap-overflow write from a crafted file), + * so the loader must reject such a header before allocating. Non-positive + * dims are rejected too. Mirrors the per-layer neighbor guard. */ +bool ray_hnsw_vec_size_valid(int64_t n_nodes, int32_t dim) { + if (n_nodes <= 0 || dim <= 0) return false; + return (uint64_t)n_nodes <= SIZE_MAX / sizeof(float) / (uint64_t)dim; +} + static ray_hnsw_t* hnsw_load_impl(const char* dir, bool use_mmap) { if (!dir) return NULL; (void)use_mmap; /* mmap optimization deferred — both paths read into memory */ @@ -915,6 +927,10 @@ static ray_hnsw_t* hnsw_load_impl(const char* dir, bool use_mmap) { hdr.M <= 0 || hdr.M_max0 <= 0 || hdr.entry_point < 0 || hdr.entry_point >= hdr.n_nodes) return NULL; + /* Reject a header whose vectors block would overflow size_t, before any + * allocation (see ray_hnsw_vec_size_valid). */ + if (!ray_hnsw_vec_size_valid(hdr.n_nodes, hdr.dim)) return NULL; + ray_hnsw_t* idx = (ray_hnsw_t*)ray_sys_alloc(sizeof(ray_hnsw_t)); if (!idx) return NULL; memset(idx, 0, sizeof(ray_hnsw_t)); diff --git a/src/store/hnsw.h b/src/store/hnsw.h index 055a3c7e7..5e1214c5e 100644 --- a/src/store/hnsw.h +++ b/src/store/hnsw.h @@ -130,4 +130,9 @@ ray_err_t ray_hnsw_save(const ray_hnsw_t* idx, const char* dir); ray_hnsw_t* ray_hnsw_load(const char* dir); ray_hnsw_t* ray_hnsw_mmap(const char* dir); +/* True iff n_nodes * dim * sizeof(float) (the vectors block) fits in size_t, + * with n_nodes and dim > 0. The loader uses it to reject a crafted header + * that would overflow the vectors allocation; exposed for direct unit test. */ +bool ray_hnsw_vec_size_valid(int64_t n_nodes, int32_t dim); + #endif /* RAY_HNSW_H */ diff --git a/test/test_embedding.c b/test/test_embedding.c index 6dea9b106..6174116fc 100644 --- a/test/test_embedding.c +++ b/test/test_embedding.c @@ -38,6 +38,7 @@ #include "store/hnsw.h" #include #include +#include #include #include @@ -983,6 +984,31 @@ static test_result_t test_hnsw_build_overflow_rejected(void) { PASS(); } +/* Directly exercises the vectors-block overflow guard that hnsw_load_impl + * applies to an untrusted header (factored into ray_hnsw_vec_size_valid). A + * crafted header can make n_nodes * dim * sizeof(float) wrap size_t, under- + * allocating the vectors buffer that the following fread overruns; the guard + * must reject it. Unit-tested at this layer rather than through ray_hnsw_load + * because a header patched to overflow is refused earlier — the huge + * node-level read fails first — so a full-load test cannot distinguish this + * path (with or without the guard the load returns NULL). */ +static test_result_t test_hnsw_vec_size_valid_guard(void) { + /* Ordinary dimensions fit. */ + TEST_ASSERT_TRUE(ray_hnsw_vec_size_valid(100000, 1536)); + /* Non-positive dims are rejected. */ + TEST_ASSERT_FALSE(ray_hnsw_vec_size_valid(0, 4)); + TEST_ASSERT_FALSE(ray_hnsw_vec_size_valid(4, 0)); + TEST_ASSERT_FALSE(ray_hnsw_vec_size_valid(-1, 4)); + /* n_nodes * dim * sizeof(float) overflows size_t → rejected. */ + TEST_ASSERT_FALSE(ray_hnsw_vec_size_valid((int64_t)1 << 40, 1 << 25)); + /* Boundary: the largest element count whose * sizeof(float) still fits, + * then one dim past it. */ + int64_t max_elems = (int64_t)(SIZE_MAX / sizeof(float)); + TEST_ASSERT_TRUE(ray_hnsw_vec_size_valid(max_elems, 1)); + TEST_ASSERT_FALSE(ray_hnsw_vec_size_valid(max_elems, 2)); + PASS(); +} + /* Trigger the maxheap_sift_down / results-replacement path in hnsw_search_layer. * * The replacement branch (lines 342-344) fires when: @@ -1979,6 +2005,7 @@ const test_entry_t embedding_entries[] = { { "embedding/hnsw_search_filter_null_accept", test_hnsw_search_filter_null_accept, emb_setup, emb_teardown }, { "embedding/hnsw_mmap_load", test_hnsw_mmap_load, emb_setup, emb_teardown }, { "embedding/hnsw_build_overflow_rejected", test_hnsw_build_overflow_rejected, emb_setup, emb_teardown }, + { "embedding/hnsw_vec_size_valid_guard", test_hnsw_vec_size_valid_guard, emb_setup, emb_teardown }, { "embedding/hnsw_search_sift_down", test_hnsw_search_sift_down, emb_setup, emb_teardown }, /* rerank coverage (S7) */