diff --git a/CHANGELOG.md b/CHANGELOG.md index 462ef4c..360981c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). -## [Unreleased] +## [1.2.0] - 2026-11-01 -Documentation and tooling only — the extension itself is unchanged from 1.1.0, so a build -from this revision produces the same binaries. +A read-through of the sources, followed by a security review of all of `src/`, turned up +ten bugs. Three of them write outside their buffers and are reachable from ordinary SQL, +so upgrading is recommended for anyone whose application lets untrusted input reach a +connection with the extension loaded. No API changed and no index needs rebuilding. ### Changed @@ -32,6 +34,63 @@ from this revision produces the same binaries. faster** than the exact scan — it does beat brute force. What holds is the comparison that mattered: `INT8` is 4x faster again at twice the index size, so TurboQuant's argument is memory rather than throughput. +- **A `NULL` key in `_sqliteai_vector` crashed the next `vector_init()`.** That table is + an ordinary writable one, and `PRIMARY KEY` on a rowid table does not imply `NOT NULL`, + so the key went to `strcmp()` unchecked. Such rows are now skipped. +- **A failed `vector_quantize()` left the extension describing an index that no longer + existed.** The quantization type, bit width, scale and offset are written into the table + context before the pass that can still fail, and the savepoint rolls the shadow table + back without them. Scans then read the chunks at the wrong stride and reported made-up + rowids and distances with no error at all. Those fields, and the TurboQuant cache, are + now restored along with the rollback. +- **A quant chunk claiming zero vectors was read past the end of its blob.** Zero is the + streaming scans' "load the next chunk" sentinel, so such a chunk slipped past the length + check beside it, which degenerates to `bytes < 0`, and one fabricated row came back. + Every count test now rejects it, and `vector_quantize_preload()` catches it at load time + rather than leaving it to query time. +- **`vector_as_*()` ignored its optional dimension argument for BLOB input**, though it is + documented as a stricter check and the `BIT` and JSON paths already enforced it. A + wrong-sized vector was stored and then skipped in silence by every scan, so it never + appeared in a result. +- **`max_memory=0` could not be set, and an unparseable value was dropped silently.** Zero + is the one value the quantizer reads as "use whatever memory the index needs", but the + parser used a `0` return to signal failure, so the two were indistinguishable. A bad + `max_memory` is now reported like every other invalid option. +- **The `INT8` and `UINT8` cosine kernels accumulated in 32 bits** in the scalar and SSE2 + backends, wrapping around past roughly 133k and 66k dimensions; NEON, AVX2 and AVX-512 + already used 64-bit accumulators. SSE2 now also computes `sqrt(a)*sqrt(b)` like every + other backend instead of `sqrtf(a*b)`, which could overflow in `float`, so all five + compute the same cosine. + +### Security + +- **Out-of-bounds write from a large `k`.** The top-k arrays were allocated with + `sqlite3_malloc()`, whose parameter is an `int`, from a `size_t` byte count. `k` comes + straight from the query, so `k=536870913` asked for `0x100000008` bytes, got 8, and the + `memset` that follows overran them. Both allocations now go through + `sqlite3_malloc64()`, so an absurd `k` is a clean `SQLITE_NOMEM`. Manifests as a crash: + the length is not controllable and the value written is zero. +- **Heap buffer overflow in JSON vector parsing.** `vector_from_json()` sized its buffer + and bounded its writes from two different numbers — the buffer from + `sqlite3_malloc((int)alloc)`, which truncated a `size_t`, and the bound from the + untruncated `alloc`. A JSON array with 2^30 commas got an 8-byte buffer and a bound of + 1073741825 elements, and the parse loop wrote `float`s of the caller's choosing straight + past it; the comma count sizes the buffer to the byte, which makes this a precise write + primitive rather than a crash. Reachable through any `vector_as_*()` and through the TEXT + query-vector argument of both scans. It took a TEXT value of about a gigabyte, so a host + built with `SQLITE_MAX_LENGTH >= 1073741824` — the limit that applies is the host's, + since this ships as a loadable extension. There is now a single capacity in 64 bits: the + allocation uses `sqlite3_malloc64()` and the loop checks against that same number. +- **Null write from an unbounded `dimension`.** The declared dimension was only checked + for being positive, so it reached `INT_MAX` and overflowed the size arithmetic + downstream. `(dim/2)*12` in the TurboQuant rotation plan wraps negative past 357913941, + which skipped the allocation of the plan's coordinate arrays and still reported the plan + as valid; `vector_init` with `dimension=400000000` followed by a TurboQuant + `vector_quantize` then crashed the process on an empty table, since the plan is built + before any row is read. Dimensions are now bounded at 2^24 where they are parsed, which + is upstream of every one of those computations — and of the signed-overflow `dim + 7` + expressions in the packing helpers. A dimension above `INT_MAX` is also rejected rather + than wrapped: `4294967300` used to come through as the dimension 4. ## [1.1.0] - 2026-08-24 diff --git a/src/distance-cpu.c b/src/distance-cpu.c index a7ce837..2916508 100644 --- a/src/distance-cpu.c +++ b/src/distance-cpu.c @@ -515,9 +515,11 @@ float uint8_distance_cosine_cpu (const void *v1, const void *v2, int n) { const uint8_t *a = (const uint8_t *)v1; const uint8_t *b = (const uint8_t *)v2; - uint32_t dot = 0; - uint32_t norm_a2 = 0; - uint32_t norm_b2 = 0; + // 64-bit for the same reason as the int8 kernel below: a product is at most 255*255, + // so uint32_t wrapped around silently past roughly 66k dimensions + uint64_t dot = 0; + uint64_t norm_a2 = 0; + uint64_t norm_b2 = 0; int i = 0; for (; i <= n - 4; i += 4) { @@ -544,7 +546,7 @@ float uint8_distance_cosine_cpu (const void *v1, const void *v2, int n) { return 1.0f; } - float cosine_similarity = dot / (sqrtf((float)norm_a2) * sqrtf((float)norm_b2)); + float cosine_similarity = (float)((double)dot / (sqrt((double)norm_a2) * sqrt((double)norm_b2))); if (cosine_similarity > 1.0f) cosine_similarity = 1.0f; if (cosine_similarity < -1.0f) cosine_similarity = -1.0f; return 1.0f - cosine_similarity; @@ -629,9 +631,12 @@ float int8_distance_cosine_cpu (const void *v1, const void *v2, int n) { const int8_t *a = (const int8_t *)v1; const int8_t *b = (const int8_t *)v2; - int32_t dot = 0; - int32_t norm_a2 = 0; - int32_t norm_b2 = 0; + // 64-bit accumulators, as the NEON/AVX2/AVX-512 kernels use: a product is at most + // 127*127, so int32_t saturated at roughly 133k dimensions and the loop wrapped + // around silently. Each group of four products still fits in 32 bits. + int64_t dot = 0; + int64_t norm_a2 = 0; + int64_t norm_b2 = 0; int i = 0; for (; i <= n - 4; i += 4) { @@ -658,7 +663,7 @@ float int8_distance_cosine_cpu (const void *v1, const void *v2, int n) { return 1.0f; } - float cosine_similarity = dot / (sqrtf((float)norm_a2) * sqrtf((float)norm_b2)); + float cosine_similarity = (float)((double)dot / (sqrt((double)norm_a2) * sqrt((double)norm_b2))); if (cosine_similarity > 1.0f) cosine_similarity = 1.0f; if (cosine_similarity < -1.0f) cosine_similarity = -1.0f; return 1.0f - cosine_similarity; diff --git a/src/distance-sse2.c b/src/distance-sse2.c index 06db316..9b16025 100644 --- a/src/distance-sse2.c +++ b/src/distance-sse2.c @@ -786,9 +786,11 @@ float uint8_distance_cosine_sse2 (const void *v1, const void *v2, int n) { _mm_storeu_si128((__m128i *)a2, acc_a2); _mm_storeu_si128((__m128i *)b2, acc_b2); - int32_t total_dot = dot[0] + dot[1] + dot[2] + dot[3]; - int32_t total_a2 = a2[0] + a2[1] + a2[2] + a2[3]; - int32_t total_b2 = b2[0] + b2[1] + b2[2] + b2[3]; + // 64-bit totals, as the NEON/AVX2/AVX-512 kernels use: the four lanes together can + // hold more than a 32-bit sum, and the scalar tail adds to the same running total + int64_t total_dot = (int64_t)dot[0] + dot[1] + dot[2] + dot[3]; + int64_t total_a2 = (int64_t)a2[0] + a2[1] + a2[2] + a2[3]; + int64_t total_b2 = (int64_t)b2[0] + b2[1] + b2[2] + b2[3]; for (; i < n; ++i) { int va = (int)a[i]; @@ -798,9 +800,9 @@ float uint8_distance_cosine_sse2 (const void *v1, const void *v2, int n) { total_b2 += vb * vb; } - float denom = sqrtf((float)total_a2 * (float)total_b2); - if (denom == 0.0f) return 1.0f; // orthogonal or zero - float cosine_sim = total_dot / denom; + double denom = sqrt((double)total_a2) * sqrt((double)total_b2); + if (denom == 0.0) return 1.0f; // orthogonal or zero + float cosine_sim = (float)((double)total_dot / denom); if (cosine_sim > 1.0f) cosine_sim = 1.0f; if (cosine_sim < -1.0f) cosine_sim = -1.0f; return 1.0f - cosine_sim; // cosine distance @@ -1003,9 +1005,11 @@ float int8_distance_cosine_sse2 (const void *v1, const void *v2, int n) { _mm_storeu_si128((__m128i *)_a, acc_a2); _mm_storeu_si128((__m128i *)_b, acc_b2); - int32_t total_dot = _d[0] + _d[1] + _d[2] + _d[3]; - int32_t total_a2 = _a[0] + _a[1] + _a[2] + _a[3]; - int32_t total_b2 = _b[0] + _b[1] + _b[2] + _b[3]; + // 64-bit totals, as the NEON/AVX2/AVX-512 kernels use: the four lanes together can + // hold more than a 32-bit sum, and the scalar tail adds to the same running total + int64_t total_dot = (int64_t)_d[0] + _d[1] + _d[2] + _d[3]; + int64_t total_a2 = (int64_t)_a[0] + _a[1] + _a[2] + _a[3]; + int64_t total_b2 = (int64_t)_b[0] + _b[1] + _b[2] + _b[3]; // Handle tail for (; i < n; ++i) { @@ -1016,9 +1020,9 @@ float int8_distance_cosine_sse2 (const void *v1, const void *v2, int n) { total_b2 += vb * vb; } - float denom = sqrtf((float)total_a2 * (float)total_b2); - if (denom == 0.0f) return 1.0f; - float cosine_sim = total_dot / denom; + double denom = sqrt((double)total_a2) * sqrt((double)total_b2); + if (denom == 0.0) return 1.0f; + float cosine_sim = (float)((double)total_dot / denom); if (cosine_sim > 1.0f) cosine_sim = 1.0f; if (cosine_sim < -1.0f) cosine_sim = -1.0f; return 1.0f - cosine_sim; diff --git a/src/sqlite-vector.c b/src/sqlite-vector.c index 9048b99..773b27d 100644 --- a/src/sqlite-vector.c +++ b/src/sqlite-vector.c @@ -77,6 +77,14 @@ SQLITE_EXTENSION_INIT1 #define MAX_TABLES 128 #define STATIC_SQL_SIZE 2048 +// A sanity bound on the declared vector dimension, which otherwise reached INT_MAX and +// overflowed the size arithmetic downstream: (dim/2)*12 in turbo_rotation_plan_init() +// wrapped negative past 357913941, which left its coordinate arrays unallocated and the +// plan reported as valid, and the (dim + 7) expressions in the packing helpers are signed +// overflow near the top of the range. 16M is some three orders of magnitude above any +// real embedding and keeps every one of those computations far inside int. +#define MAX_VECTOR_DIMENSION (1 << 24) + #define INT64_TO_INT8PTR(_val, _ptr) do { \ (_ptr)[0] = (int8_t)(((_val) >> 0) & 0xFF); \ (_ptr)[1] = (int8_t)(((_val) >> 8) & 0xFF); \ @@ -552,7 +560,11 @@ static int sqlite_unserialize (sqlite3_context *context, table_context *ctx) { if (rc == SQLITE_DONE) {rc = SQLITE_OK; break;} if (rc != SQLITE_ROW) break; + // _sqliteai_vector is an ordinary writable table, and PRIMARY KEY on a rowid + // table does not imply NOT NULL: a row with a NULL key used to reach strcmp() const char *key = (const char *)sqlite3_column_text(vm, 0); + if (!key) continue; + if (strcmp(key, OPTION_KEY_QUANTTYPE) == 0) { ctx->options.q_type = (vector_qtype)sqlite3_column_int(vm, 1); continue; @@ -1584,10 +1596,15 @@ static bool parse_keyvalue_string (sqlite3_context *context, const char *str, ke return true; } -static uint64_t human_to_number (const char *s) { +// Returns false when the string is not a size at all, and writes the value out otherwise. +// The previous version signalled failure by returning 0, which also made a literal "0" +// unparseable - and zero is the one value the quantizer reads as "use whatever memory the +// index needs", so that setting could not be expressed through the option string. +static bool human_to_number (const char *s, uint64_t *out) { char *end = NULL; double d = strtod(s, &end); - if ((d == 0) || (d == HUGE_VAL)) return 0; + if (end == s) return false; // no digits at all + if (!isfinite(d)) return false; // overflowed to HUGE_VAL, or a NaN literal // skip whitespace before suffix SKIP_SPACES(end); @@ -1596,11 +1613,12 @@ static uint64_t human_to_number (const char *s) { if (strncasecmp(end, "KB", 2) == 0) d *= 1024; else if (strncasecmp(end, "MB", 2) == 0) d *= 1024 * 1024; else if (strncasecmp(end, "GB", 2) == 0) d *= 1024 * 1024 * 1024; - else if (*end != 0) return 0; // invalid suffix + else if (*end != 0) return false; // invalid suffix // sanity check - if (d < 0 || d > (double)INT64_MAX) return 0; - return (uint64_t)d; + if (d < 0 || d > (double)INT64_MAX) return false; + *out = (uint64_t)d; + return true; } bool vector_keyvalue_callback (sqlite3_context *context, void *xdata, const char *key, int key_len, const char *value, int value_len) { @@ -1626,9 +1644,13 @@ bool vector_keyvalue_callback (sqlite3_context *context, void *xdata, const char } if (KEY_MATCH(OPTION_KEY_DIMENSION)) { - int dimension = (int)strtol(buffer, NULL, 0); - if (dimension <= 0) return context_result_error(context, SQLITE_ERROR, "Invalid vector dimension: expected a positive integer, got '%s'", buffer); - options->v_dim = dimension; + // kept as a long until it has been range-checked: casting strtol()'s result to + // int first silently wrapped, so "4294967300" came through as the dimension 4 + long dimension = strtol(buffer, NULL, 0); + if (dimension <= 0 || dimension > MAX_VECTOR_DIMENSION) { + return context_result_error(context, SQLITE_ERROR, "Invalid vector dimension: expected an integer between 1 and %d, got '%s'", MAX_VECTOR_DIMENSION, buffer); + } + options->v_dim = (int)dimension; return true; } @@ -1639,8 +1661,11 @@ bool vector_keyvalue_callback (sqlite3_context *context, void *xdata, const char } if (KEY_MATCH(OPTION_KEY_MAXMEMORY)) { - uint64_t max_memory = human_to_number(buffer); - if (max_memory > 0) options->max_memory = max_memory; + uint64_t max_memory = 0; + // reported like every other unparseable option instead of being dropped, and 0 is + // now a value you can ask for: it means "use as much memory as the index needs" + if (!human_to_number(buffer, &max_memory)) return context_result_error(context, SQLITE_ERROR, "Invalid max_memory value: expected a size such as 64MB, got '%s'", buffer); + options->max_memory = max_memory; return true; } @@ -1901,9 +1926,10 @@ static int vector_rebuild_quantization (sqlite3_context *context, const char *ta // max_memory == 0 means use all required memory if (max_memory == 0) { sqlite3_snprintf(sizeof(sql), sql, "SELECT COUNT(*) FROM \"%w\";", table_name); - int64_t count = sqlite_read_int64(db, sql); - max_memory = (count == 0) ? DEFAULT_MAX_MEMORY : (uint64_t)count * (uint64_t)q_size; - if (count <= 0) { + // named apart from the caller's out-parameter, which it used to shadow + int64_t nrows = sqlite_read_int64(db, sql); + max_memory = (nrows == 0) ? DEFAULT_MAX_MEMORY : (uint64_t)nrows * (uint64_t)q_size; + if (nrows <= 0) { // no vectors t_ctx->options.q_type = (qtype == VECTOR_QUANT_AUTO) ? VECTOR_QUANT_U8BIT : qtype; t_ctx->options.q_bits = q_bits; @@ -2197,7 +2223,9 @@ static void vector_quantize_preload (sqlite3_context *context, int argc, sqlite3 int bytes = sqlite3_column_bytes(vm, 1); const uint8_t *data = (const uint8_t *)sqlite3_column_blob(vm, 1); - if (!data || n < 0 || bytes < 0 || (sqlite3_int64)bytes > required - seek) { + // n <= 0: every chunk this extension writes holds at least one vector, so a + // chunk claiming none is malformed and is caught here rather than at query time + if (!data || n <= 0 || bytes < 0 || (sqlite3_int64)bytes > required - seek) { rc = SQLITE_CORRUPT; break; } @@ -2241,6 +2269,16 @@ static int vector_quantize (sqlite3_context *context, const char *table_name, co char sql[STATIC_SQL_SIZE]; sqlite3 *db = sqlite3_context_db_handle(context); + // vector_rebuild_quantization() writes q_type/q_bits/scale/offset into t_ctx before + // the pass that can still fail, and the cleanup path below rolls the shadow table + // back. Snapshot them here so the in-memory description of the index goes back too: + // otherwise a failed call left the context describing a layout the table no longer + // has, and the next scan read the chunks at the wrong stride - wrong rowids, wrong + // distances, no error. Taken before the first goto so the label can always restore. + const vector_options saved_options = t_ctx->options; + const float saved_scale = t_ctx->scale; + const float saved_offset = t_ctx->offset; + bool savepoint_open = false; rc = sqlite3_exec(db, "SAVEPOINT quantize;", NULL, NULL, NULL); if (rc != SQLITE_OK) goto quantize_cleanup; @@ -2294,6 +2332,22 @@ quantize_cleanup: { sqlite3_exec(db, "RELEASE quantize;", NULL, NULL, NULL); } + // put the context back in step with the table the rollback restored. The turbo + // cache is keyed by (bits, dim) and rebuild() drops it whenever it quantizes to + // anything else, so a restored TURBO index needs it built again; if that fails + // the cache simply stays absent and the scans report SQLITE_MISUSE rather than + // reading at the wrong stride. + sqlite3_mutex_enter(qmutex); + t_ctx->options = saved_options; + t_ctx->scale = saved_scale; + t_ctx->offset = saved_offset; + if (t_ctx->options.q_type == VECTOR_QUANT_TURBO) { + if (table_context_ensure_turbo_codebook(t_ctx, t_ctx->options.q_bits, t_ctx->options.v_dim) == SQLITE_OK) { + table_context_ensure_turbo_plan(t_ctx, t_ctx->options.v_dim); + } + } + sqlite3_mutex_leave(qmutex); + // only replace the message when SQLite actually has one: the callees set their own // through context_result_error, and overwriting it reported "not an error" if (errmsg) { @@ -2382,22 +2436,33 @@ static void *vector_from_json (sqlite3_context *context, sqlite3_vtab *vtab, vec json++; - // count number of commas to estimate (over-estimate) the number of entries in the json - int estimated_count = 0; + // count number of commas to estimate (over-estimate) the number of entries in the + // json: a well-formed array of n elements carries n-1 commas, and a malformed one + // only ever parses into fewer. 64-bit, because a comma per byte of a maximum-length + // TEXT value overflows an int counter. + int64_t capacity = 1; for (const char *p = json; *p; ++p) { - if (*p == ',') estimated_count++; + if (*p == ',') ++capacity; } - + // allocate blob // For BIT type, each JSON element is a single bit, pack 8 per byte size_t item_size = vector_type_to_size(type); - size_t alloc = (type == VECTOR_TYPE_BIT) ? ((estimated_count + 1) + 7) / 8 : (estimated_count + 1) * item_size; - blob = sqlite3_malloc((int)alloc); + // sqlite3_malloc() takes an int, so this byte count used to be truncated from a + // size_t while the write bound below was still derived from the untruncated value: a + // JSON array with 2^30 commas got an 8-byte buffer and a billion-element bound, and + // the parse loop wrote straight past it. The bound is now this same capacity rather + // than a figure recovered from the allocation, and sqlite3_malloc64() refuses + // anything near 2GB instead of wrapping. + sqlite3_uint64 alloc = (type == VECTOR_TYPE_BIT) + ? (sqlite3_uint64)((capacity + 7) / 8) + : (sqlite3_uint64)capacity * (sqlite3_uint64)item_size; + blob = sqlite3_malloc64(alloc); if (!blob) { return sqlite_common_set_error(context, vtab, SQLITE_NOMEM, "Out of memory: unable to allocate %lld bytes for BLOB buffer", (long long)alloc); } if (type == VECTOR_TYPE_BIT) { - memset(blob, 0, alloc); // Initialize to zero for bit packing + memset(blob, 0, (size_t)alloc); // Initialize to zero for bit packing } // typed pointers @@ -2427,9 +2492,9 @@ static void *vector_from_json (sqlite3_context *context, sqlite3_vtab *vtab, vec return sqlite_common_set_error(context, vtab, SQLITE_ERROR, "Malformed JSON: expected a number at position %d (found '%c')", (int)(p - json) + 1, *p ? *p : '?'); } - // check bounds - int max_count = (type == VECTOR_TYPE_BIT) ? (int)(alloc * 8) : (int)(alloc / item_size); - if (count >= max_count) { + // check bounds against the capacity the buffer was sized for, never against a + // count recovered from the allocation + if ((int64_t)count >= capacity) { sqlite3_free(blob); return sqlite_common_set_error(context, vtab, SQLITE_ERROR, "Too many elements in JSON array"); } @@ -2509,7 +2574,10 @@ static void *vector_from_json (sqlite3_context *context, sqlite3_vtab *vtab, vec return sqlite_common_set_error(context, vtab, SQLITE_ERROR, "Invalid JSON vector dimension: expected %d but found %d", dimension, count); } - if (size) *size = (type == VECTOR_TYPE_BIT) ? (int)((count + 7) / 8) : (int)(count * item_size); + // 64-bit arithmetic here too: the allocation above already caps count low enough that + // neither expression can overflow, but computing it in int left that guarantee resting + // on the allocator's limit rather than on anything stated locally + if (size) *size = (type == VECTOR_TYPE_BIT) ? (int)(((int64_t)count + 7) / 8) : (int)((int64_t)count * (int64_t)item_size); return blob; } @@ -2520,7 +2588,15 @@ static void vector_as_type (sqlite3_context *context, vector_type type, int argc // dimension is an optional argument int dimension = (argc == 2) ? sqlite3_value_int(argv[1]) : 0; - + + // held to the same bound as the dimension option, for the same reason: the size + // checks below go through vector_bytes_for_dim(), whose dim * item_size wraps where + // size_t is 32 bits, and a wrapped product would let a wrong-sized blob match + if (dimension > MAX_VECTOR_DIMENSION) { + context_result_error(context, SQLITE_ERROR, "Invalid dimension: expected an integer between 1 and %d, got %d", MAX_VECTOR_DIMENSION, dimension); + return; + } + if (value_type == SQLITE_BLOB) { if (type == VECTOR_TYPE_BIT) { // For bit vectors, any size is valid (dimensions = size * 8, minus padding) @@ -2529,19 +2605,33 @@ static void vector_as_type (sqlite3_context *context, vector_type type, int argc size_t expected_size = vector_bytes_for_dim(type, dimension); if ((size_t)value_size != expected_size) { context_result_error(context, SQLITE_ERROR, - "Invalid BLOB size for format '%s': expected %d bytes for %d dimensions (got %d bytes)", - vector_type_to_name(type), (int)expected_size, dimension, value_size); + "Invalid BLOB size for format '%s': expected %lld bytes for %d dimensions (got %d bytes)", + vector_type_to_name(type), (long long)expected_size, dimension, value_size); return; } } } else { - // the only check we can perform is that the blob size is an exact multiplier of the vector type + // without a dimension the only check we can perform is that the blob size is + // an exact multiplier of the vector type if (value_size % vector_type_to_size(type) != 0) { context_result_error(context, SQLITE_ERROR, "Invalid BLOB size for format '%s': size must be a multiple of %d bytes", vector_type_to_name(type), vector_type_to_size(type)); return; } + + // with one, hold the blob to it, as the BIT branch above and the JSON branch + // below already do: a wrong-sized vector is otherwise stored happily and then + // skipped in silence by every scan, so it never appears in any result + if (dimension > 0) { + size_t expected_size = vector_bytes_for_dim(type, dimension); + if ((size_t)value_size != expected_size) { + context_result_error(context, SQLITE_ERROR, + "Invalid BLOB size for format '%s': expected %lld bytes for %d dimensions (got %d bytes)", + vector_type_to_name(type), (long long)expected_size, dimension, value_size); + return; + } + } } sqlite3_result_value(context, value); @@ -2736,22 +2826,29 @@ static int vCursorFilterCommon (sqlite3_vtab_cursor *cur, int idxNum, const char } if (c->row_count != k) { + // sqlite3_malloc() takes an int: k * sizeof() is a size_t and k comes straight + // from the query, so the byte count was silently truncated - k=536870913 asked + // for 0x100000008 bytes and got 8, which the memset below then overran. + // sqlite3_malloc64() refuses anything near 2GB, so an absurd k is a clean NOMEM. + // + // row_count also has to drop to zero before either allocation: it is what tells + // the next filter whether these arrays can be reused, and a cursor that failed + // half way through used to keep the old count next to one freed array. if (c->rowids) sqlite3_free(c->rowids); - c->rowids = (int64_t *)sqlite3_malloc(k * sizeof(int64_t)); - if (c->rowids == NULL) { - if (vector_allocated) sqlite3_free((void *)vector); - return SQLITE_NOMEM; - } - if (c->distance) sqlite3_free(c->distance); - c->distance = (double *)sqlite3_malloc(k * sizeof(double)); - if (c->distance == NULL) { + c->rowids = NULL; + c->distance = NULL; + c->row_count = 0; + + c->rowids = (int64_t *)sqlite3_malloc64((sqlite3_uint64)k * sizeof(int64_t)); + c->distance = (double *)sqlite3_malloc64((sqlite3_uint64)k * sizeof(double)); + if (c->rowids == NULL || c->distance == NULL) { if (vector_allocated) sqlite3_free((void *)vector); return SQLITE_NOMEM; } } - memset(c->rowids, 0, k * sizeof(int64_t)); + memset(c->rowids, 0, (size_t)k * sizeof(int64_t)); for (int i=0; idistance[i] = INFINITY; c->size = 0; @@ -2964,7 +3061,13 @@ static int vFullScanCursorNext (sqlite3_vtab_cursor *cur){ c->stream.data = (uint8_t *)sqlite3_column_blob(vm, 1); c->stream.data_bytes = sqlite3_column_bytes(vm, 1); c->stream.dindex = 0; - if (c->stream.data == NULL || c->stream.dcounter < 0 || c->stream.data_bytes < 0 || (sqlite3_uint64)c->stream.data_bytes < (sqlite3_uint64)c->stream.dcounter * (sqlite3_uint64)total_stride) { + // dcounter == 0 is this loop's "load the next chunk" sentinel, so a chunk + // that claims zero vectors has to be rejected and not merely skipped: the + // length check below degenerates to "bytes < 0" and the row would then be + // read from past the end of the blob. vector_serialize_quantization() never + // writes such a chunk, so seeing one means the shadow table was tampered + // with - the same thing the counter and length tests here already guard. + if (c->stream.data == NULL || c->stream.dcounter <= 0 || c->stream.data_bytes < 0 || (sqlite3_uint64)c->stream.data_bytes < (sqlite3_uint64)c->stream.dcounter * (sqlite3_uint64)total_stride) { return SQLITE_CORRUPT; } } @@ -3045,7 +3148,9 @@ static int vFullScanCursorNext (sqlite3_vtab_cursor *cur){ c->stream.data = (uint8_t *)sqlite3_column_blob(vm, 1); c->stream.data_bytes = sqlite3_column_bytes(vm, 1); c->stream.dindex = 0; // reset index for the new chunk - if (c->stream.data == NULL || c->stream.dcounter < 0 || c->stream.data_bytes < 0 || + // dcounter <= 0, not < 0: zero is this loop's "chunk exhausted" sentinel, so a + // chunk claiming no vectors would otherwise be read past the end of its blob + if (c->stream.data == NULL || c->stream.dcounter <= 0 || c->stream.data_bytes < 0 || (sqlite3_uint64)c->stream.data_bytes < (sqlite3_uint64)c->stream.dcounter * (sqlite3_uint64)total_stride) { return SQLITE_CORRUPT; } @@ -3206,9 +3311,11 @@ static int vQuantRunMemory(vFullScanCursor *c, const preload_index *idx, uint8_t const size_t total_stride = rowid_size + vector_size; // the preloaded index is built from an ordinary writable table, so never trust its - // row count against its length (the TurboQuant paths already do this) + // row count against its length (the TurboQuant paths already do this). A count of + // zero over a non-empty buffer is malformed too - preload refuses to build an index + // with no bytes at all, so every chunk in it must claim at least one vector const sqlite3_int64 data_bytes = idx->bytes; - if (!data || counter < 0 || data_bytes < 0 || (sqlite3_uint64)data_bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { + if (!data || counter <= 0 || data_bytes < 0 || (sqlite3_uint64)data_bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { sqlite_vtab_set_error(c->base.pVtab, "Corrupted quantization data preloaded for '%s.%s'", c->table->t_name, c->table->c_name); return SQLITE_CORRUPT; } @@ -3276,7 +3383,9 @@ static int vTurboRunPackedRows (vFullScanCursor *c, const uint8_t *data, sqlite3 vector_distance distance_type = c->table->options.v_distance; size_t packed_bytes = turbo_bytes_for_dim(dim, bits); size_t total_stride = sizeof(int64_t) + sizeof(float) + packed_bytes; - if (!data || counter < 0 || data_bytes < 0 || (sqlite3_uint64)data_bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { + // a chunk that carries bytes but claims no vectors is malformed, same as one whose + // count does not fit its length: reject it here as the streaming paths now do + if (!data || counter <= 0 || data_bytes < 0 || (sqlite3_uint64)data_bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { return SQLITE_CORRUPT; } @@ -3462,7 +3571,9 @@ static int vQuantRun (sqlite3 *db, vFullScanCursor *c, const void *v1, int v1siz int counter = sqlite3_column_int(vm, 0); uint8_t *data = (uint8_t *)sqlite3_column_blob(vm, 1); int bytes = sqlite3_column_bytes(vm, 1); - if (!data || counter < 0 || bytes < 0 || (sqlite3_uint64)bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { + // counter <= 0: a chunk with bytes but no vectors is as malformed as one whose + // count overruns its length, and the streaming paths reject it too + if (!data || counter <= 0 || bytes < 0 || (sqlite3_uint64)bytes < (sqlite3_uint64)counter * (sqlite3_uint64)total_stride) { sqlite_vtab_set_error(c->base.pVtab, "Corrupted quantization data for '%s.%s'", c->table->t_name, c->table->c_name); sqlite3_finalize(vm); sqlite3_free(v); diff --git a/src/sqlite-vector.h b/src/sqlite-vector.h index 2a6777f..028f815 100644 --- a/src/sqlite-vector.h +++ b/src/sqlite-vector.h @@ -24,7 +24,7 @@ extern "C" { #endif -#define SQLITE_VECTOR_VERSION "1.1.2" +#define SQLITE_VECTOR_VERSION "1.2.0" SQLITE_VECTOR_API int sqlite3_vector_init (sqlite3 *db, char **pzErrMsg, const sqlite3_api_routines *pApi); diff --git a/test/test_vector.c b/test/test_vector.c index 1cec092..93ba354 100644 --- a/test/test_vector.c +++ b/test/test_vector.c @@ -1007,6 +1007,280 @@ static void test_distance_functions_hamming(sqlite3 *db) { } } +/* ---------- Test: malformed input and failure paths ---------- */ + +/* Like exec_scan_sql but keeps the step() return code, which the tests below need in + order to tell "rejected cleanly" from "produced a row it should not have". */ +static int run_scan(sqlite3 *db, const char *sql, scan_result *r) { + sqlite3_stmt *stmt = NULL; + int rc = sqlite3_prepare_v2(db, sql, -1, &stmt, NULL); + if (rc != SQLITE_OK) return rc; + + while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) { + if (r->count < 64) { + r->ids[r->count] = sqlite3_column_int(stmt, 0); + r->distances[r->count] = sqlite3_column_double(stmt, 1); + } + r->count++; + } + + sqlite3_finalize(stmt); + return (rc == SQLITE_DONE) ? SQLITE_OK : rc; +} + +static void test_malformed_input(sqlite3 *db) { + printf("\n=== Malformed input and failure paths ===\n"); + char sql[1024]; + + /* + * A k whose byte count overflows an int. sqlite3_malloc() takes an int, so + * k=536870913 asked for 0x100000008 bytes, got 8, and the memset that follows + * overran the top-k arrays. It has to fail cleanly, and leave a usable cursor. + */ + { + const char *tbl = "tbad_bigk"; + if (setup_table(db, tbl, "f32", "L2", 4, float_vecs, float_nvecs) != 0) { + ASSERT(0, "huge k setup"); + } else { + scan_result r = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_full_scan('%s','v',vector_as_f32('%s'),536870913);", + tbl, float_query); + int rc = run_scan(db, sql, &r); + ASSERT(rc != SQLITE_OK, "huge k is rejected instead of overflowing the top-k arrays"); + + scan_result r2 = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_full_scan('%s','v',vector_as_f32('%s'),3);", + tbl, float_query); + rc = run_scan(db, sql, &r2); + ASSERT(rc == SQLITE_OK && r2.count == 3, "a sane k still works after a huge one"); + } + } + + /* + * _sqliteai_vector is an ordinary writable table and PRIMARY KEY on a rowid table + * does not imply NOT NULL, so a row with a NULL key used to reach strcmp(NULL). + */ + { + const char *tbl = "tbad_nullkey"; + snprintf(sql, sizeof(sql), "CREATE TABLE \"%s\" (id INTEGER PRIMARY KEY, v BLOB);", tbl); + exec_sql(db, sql); + snprintf(sql, sizeof(sql), + "INSERT INTO _sqliteai_vector (tblname,colname,key,value) VALUES ('%s','v',NULL,1);", tbl); + int rc = exec_sql(db, sql); + ASSERT(rc == SQLITE_OK, "a NULL metadata key can be stored"); + + snprintf(sql, sizeof(sql), + "SELECT vector_init('%s','v','type=f32,dimension=4,distance=L2');", tbl); + rc = exec_sql(db, sql); + ASSERT(rc == SQLITE_OK, "vector_init skips a NULL metadata key instead of crashing"); + } + + /* + * A quant chunk that claims zero vectors. Zero is the streaming scans' "load the + * next chunk" sentinel, so such a chunk slipped past the length check and one row + * was read from past the end of its blob. + */ + { + const char *tbl = "tbad_zerocount"; + if (setup_table(db, tbl, "f32", "L2", 4, float_vecs, float_nvecs) != 0) { + ASSERT(0, "zero-count chunk setup"); + } else { + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=UINT8');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "zero-count chunk: quantize succeeds"); + + scan_result before = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'));", + tbl, float_query); + ASSERT(run_scan(db, sql, &before) == SQLITE_OK, "zero-count chunk: scan works before tampering"); + + snprintf(sql, sizeof(sql), + "INSERT INTO \"vector0_%s_v\" (rowid1,rowid2,counter,data) VALUES (0,0,0,x'00');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "zero-count chunk can be stored"); + + scan_result streaming = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'));", + tbl, float_query); + int rc = run_scan(db, sql, &streaming); + ASSERT(rc != SQLITE_OK, "streaming scan rejects a chunk claiming zero vectors"); + ASSERT(streaming.count <= before.count, "streaming scan invents no extra row"); + + scan_result topk = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'),5);", + tbl, float_query); + rc = run_scan(db, sql, &topk); + ASSERT(rc != SQLITE_OK, "top-k scan rejects a chunk claiming zero vectors"); + } + } + + /* + * A vector_quantize that fails part way through. The savepoint rolls the shadow + * table back, so the in-memory q_type/scale/offset have to go back too: otherwise + * the next scan reads the chunks at the wrong stride and reports made-up rowids. + * 1BIT skips the min/max pass, so the undersized blob is caught in the later one. + */ + { + const char *tbl = "tbad_rollback"; + if (setup_table(db, tbl, "f32", "L2", 4, float_vecs, float_nvecs) != 0) { + ASSERT(0, "quantize rollback setup"); + } else { + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=UINT8');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "quantize rollback: UINT8 quantize succeeds"); + + scan_result before = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'),5) ORDER BY distance;", + tbl, float_query); + ASSERT(run_scan(db, sql, &before) == SQLITE_OK, "quantize rollback: scan works before"); + + snprintf(sql, sizeof(sql), "INSERT INTO \"%s\" (id, v) VALUES (9999, x'0001');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "quantize rollback: undersized blob stored"); + + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=1BIT');", tbl); + ASSERT(exec_sql(db, sql) != SQLITE_OK, "quantize rollback: 1BIT quantize fails on the bad blob"); + + snprintf(sql, sizeof(sql), "DELETE FROM \"%s\" WHERE id = 9999;", tbl); + exec_sql(db, sql); + + scan_result after = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'),5) ORDER BY distance;", + tbl, float_query); + ASSERT(run_scan(db, sql, &after) == SQLITE_OK, "quantize rollback: scan still works after the failure"); + ASSERT(after.count == before.count, "quantize rollback: same number of rows as before"); + + int same = (after.count == before.count); + for (int i = 0; same && i < after.count; i++) { + if (after.ids[i] != before.ids[i]) same = 0; + if (after.distances[i] != before.distances[i]) same = 0; + } + ASSERT(same, "quantize rollback: same rowids and distances as before the failed call"); + } + } + + /* + * The optional dimension argument has to hold a BLOB to the size it names, as it + * already did for TEXT and for BIT: a wrong-sized vector is otherwise stored and + * then skipped in silence by every scan. + */ + { + sqlite3_stmt *stmt = NULL; + int rc = sqlite3_prepare_v2(db, "SELECT length(vector_as_f32(x'0000000000000000', 4));", -1, &stmt, NULL); + if (rc == SQLITE_OK) rc = sqlite3_step(stmt); + ASSERT(rc != SQLITE_ROW, "vector_as_f32 rejects a BLOB that does not match the given dimension"); + sqlite3_finalize(stmt); + + stmt = NULL; + rc = sqlite3_prepare_v2(db, "SELECT length(vector_as_f32(x'0000000000000000', 2));", -1, &stmt, NULL); + if (rc == SQLITE_OK) rc = sqlite3_step(stmt); + ASSERT(rc == SQLITE_ROW && sqlite3_column_int(stmt, 0) == 8, "vector_as_f32 accepts a BLOB of the right dimension"); + sqlite3_finalize(stmt); + + /* + * The size in the rejection message is a size_t behind a %lld, so it has to be + * reported whole. context_result_error() carries no printf format attribute, so + * nothing but a runtime check covers the conversion. + */ + stmt = NULL; + rc = sqlite3_prepare_v2(db, "SELECT vector_as_f32(x'00000000', 16777216);", -1, &stmt, NULL); + if (rc == SQLITE_OK) rc = sqlite3_step(stmt); + ASSERT(rc != SQLITE_ROW, "vector_as_f32 rejects a 4-byte BLOB offered as 16777216 dimensions"); + { + const char *msg = sqlite3_errmsg(db); + ASSERT(msg && strstr(msg, "67108864") != NULL, "the rejection reports the expected size"); + if (!msg || !strstr(msg, "67108864")) printf(" msg: %s\n", msg ? msg : "(none)"); + } + sqlite3_finalize(stmt); + + /* past the dimension bound it is refused before any size is computed at all */ + stmt = NULL; + rc = sqlite3_prepare_v2(db, "SELECT vector_as_f32(x'00000000', 1073741824);", -1, &stmt, NULL); + if (rc == SQLITE_OK) rc = sqlite3_step(stmt); + ASSERT(rc != SQLITE_ROW, "vector_as_f32 rejects a dimension past the bound"); + sqlite3_finalize(stmt); + + stmt = NULL; + rc = sqlite3_prepare_v2(db, "SELECT length(vector_as_f32(x'0000000000000000'));", -1, &stmt, NULL); + if (rc == SQLITE_OK) rc = sqlite3_step(stmt); + ASSERT(rc == SQLITE_ROW && sqlite3_column_int(stmt, 0) == 8, "vector_as_f32 still accepts a BLOB with no dimension given"); + sqlite3_finalize(stmt); + } + + /* + * An unbounded dimension overflowed the size arithmetic downstream. The one that + * crashed outright: (dim/2)*12 in turbo_rotation_plan_init() wraps negative past + * 357913941, which skipped the allocation of the coordinate arrays and still + * reported the plan as valid, so building a TurboQuant index on an EMPTY table + * wrote through a NULL pointer. The dimension is now range-checked where it is + * parsed, which is upstream of every one of those computations. + */ + { + const char *tbl = "tbad_dim"; + snprintf(sql, sizeof(sql), "CREATE TABLE \"%s\" (id INTEGER PRIMARY KEY, v BLOB);", tbl); + exec_sql(db, sql); + + struct { const char *dim; int ok; const char *what; } cases[] = { + {"4", 1, "a small dimension is accepted"}, + {"16777216", 1, "the largest allowed dimension is accepted"}, + {"16777217", 0, "one past the bound is rejected"}, + {"400000000", 0, "the dimension that overflowed the TurboQuant plan is rejected"}, + {"2147483647", 0, "INT_MAX is rejected"}, + /* (int)strtol() used to wrap this to 4 and accept it as a valid dimension */ + {"4294967300", 0, "a value above INT_MAX is rejected, not wrapped"}, + }; + + for (unsigned i = 0; i < sizeof(cases) / sizeof(*cases); i++) { + char msg[160]; + /* a fresh column name each time: vector_init keeps the first dimension it + is given for a table/column pair and rejects later disagreements */ + snprintf(sql, sizeof(sql), "ALTER TABLE \"%s\" ADD COLUMN c%u BLOB;", tbl, i); + exec_sql(db, sql); + snprintf(sql, sizeof(sql), "SELECT vector_init('%s','c%u','type=f32,dimension=%s');", tbl, i, cases[i].dim); + int rc = exec_sql(db, sql); + snprintf(msg, sizeof(msg), "dimension=%s: %s", cases[i].dim, cases[i].what); + ASSERT(cases[i].ok ? (rc == SQLITE_OK) : (rc != SQLITE_OK), msg); + } + + /* and the quantize that used to crash is now unreachable: init refuses first */ + snprintf(sql, sizeof(sql), "ALTER TABLE \"%s\" ADD COLUMN turbo BLOB;", tbl); + exec_sql(db, sql); + snprintf(sql, sizeof(sql), "SELECT vector_init('%s','turbo','type=f32,dimension=400000000,distance=COSINE');", tbl); + ASSERT(exec_sql(db, sql) != SQLITE_OK, "the TurboQuant crash case is refused at vector_init"); + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','turbo','qtype=TURBO');", tbl); + ASSERT(exec_sql(db, sql) != SQLITE_OK, "and vector_quantize on it reports no context instead of crashing"); + } + + /* + * max_memory: 0 is a real setting ("use whatever the index needs") and used to be + * indistinguishable from a parse failure, which was itself silently ignored. + */ + { + const char *tbl = "tbad_maxmemory"; + if (setup_table(db, tbl, "f32", "L2", 4, float_vecs, float_nvecs) != 0) { + ASSERT(0, "max_memory setup"); + } else { + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=UINT8,max_memory=0');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "max_memory=0 is accepted"); + + scan_result r = {0}; + snprintf(sql, sizeof(sql), + "SELECT id, distance FROM vector_quantize_scan('%s','v',vector_as_f32('%s'),5);", + tbl, float_query); + ASSERT(run_scan(db, sql, &r) == SQLITE_OK && r.count == 5, "max_memory=0 quantizes a usable index"); + + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=UINT8,max_memory=banana');", tbl); + ASSERT(exec_sql(db, sql) != SQLITE_OK, "an unparseable max_memory is reported, not ignored"); + + snprintf(sql, sizeof(sql), "SELECT vector_quantize('%s','v','qtype=UINT8,max_memory=1MB');", tbl); + ASSERT(exec_sql(db, sql) == SQLITE_OK, "max_memory with a unit suffix is accepted"); + } + } +} + /* ---------- Main ---------- */ int main(void) { @@ -1218,6 +1492,9 @@ int main(void) { test_distance_functions_hamming(db); } + /* 6. malformed input and failure paths */ + test_malformed_input(db); + sqlite3_close(db);