From c37b3e3c420a3a372d2be040554a5ebb2f0c341d Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:22:50 -0700 Subject: [PATCH] Fix vec0 parser token-type guards: && should be || (20 sites) Every "next token must be X" guard in the vec0 column/option-definition parser (vec0_parse_table_option, vec0_parse_partition_key_definition, vec0_parse_auxiliary_column_definition, vec0_parse_primary_key_definition, vec0_parse_vector_column) is written as: rc = vec0_scanner_next(&scanner, &token); if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_X) { return SQLITE_EMPTY; // or SQLITE_ERROR } This is backwards in two ways: - When a token IS present (the common case), `rc != VEC0_TOKEN_RESULT_SOME` is false, and `false && anything` is unconditionally false -- the token_type check never runs at all. A present-but-wrong-type token (e.g. a digit where an identifier or '=' was expected) is silently accepted and its (wrong) start/end pointers get used as if they were correct. - When no token is present (EOF), `token` is left uninitialised by the scanner. `true && token.token_type != TOKEN_TYPE_X` then reads that uninitialised value, so whether the guard fires is undefined -- and on the branch where it doesn't, the code falls through and dereferences uninitialised `token.start`/`token.end`. This is what the nightly valgrind memcheck job (not ASan/UBSan, which don't catch uninitialised reads) reports as ~20 "use of uninitialised value" errors in vec0_parse_* and the sqlite3_strnicmp calls they feed. Every one of these guards should be `||`: bail if there's no token, OR the token that is there is the wrong type. Several sibling checks in the same functions already use `||` correctly, which is how the drift is visible -- this makes all 20 sites consistent with those. Fix: mechanical && -> || flip at all 20 sites. No call site needed the buggy behavior; each one's job is purely "reject if this isn't token X". Testing: added test_vec0_column_option_malformed_value to tests/test-loadable.py, asserting that a distance_metric option with a missing value (EOF right after '=') now deterministically raises rather than falling through with an uninitialised token. --- sqlite-vec.c | 40 ++++++++++++++++++++-------------------- tests/test-loadable.py | 12 ++++++++++++ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/sqlite-vec.c b/sqlite-vec.c index 7af3b6a7..b53fb97a 100644 --- a/sqlite-vec.c +++ b/sqlite-vec.c @@ -2206,7 +2206,7 @@ int vec0_parse_table_option(const char *source, int source_length, vec0_scanner_init(&scanner, source, source_length); rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2214,12 +2214,12 @@ int vec0_parse_table_option(const char *source, int source_length, keyLength = token.end - token.start; rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_EQ) { + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_EQ) { return SQLITE_EMPTY; } rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || !((token.token_type == TOKEN_TYPE_IDENTIFIER) || (token.token_type == TOKEN_TYPE_DIGIT))) { return SQLITE_ERROR; @@ -2262,7 +2262,7 @@ int vec0_parse_partition_key_definition(const char *source, int source_length, // Check first token is identifier, will be the column name int rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2272,7 +2272,7 @@ int vec0_parse_partition_key_definition(const char *source, int source_length, // Check the next token matches "text" or "integer", as column type rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2289,7 +2289,7 @@ int vec0_parse_partition_key_definition(const char *source, int source_length, // Check the next token is identifier and matches "partition" rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2299,7 +2299,7 @@ int vec0_parse_partition_key_definition(const char *source, int source_length, // Check the next token is identifier and matches "key" rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2345,7 +2345,7 @@ int vec0_parse_auxiliary_column_definition(const char *source, int source_length } rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2355,7 +2355,7 @@ int vec0_parse_auxiliary_column_definition(const char *source, int source_length // Check the next token matches "text" or "integer", as column type rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2477,7 +2477,7 @@ int vec0_parse_primary_key_definition(const char *source, int source_length, // Check first token is identifier, will be the column name int rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2487,7 +2487,7 @@ int vec0_parse_primary_key_definition(const char *source, int source_length, // Check the next token matches "text" or "integer", as column type rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2504,7 +2504,7 @@ int vec0_parse_primary_key_definition(const char *source, int source_length, // Check the next token is identifier and matches "primary" rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2514,7 +2514,7 @@ int vec0_parse_primary_key_definition(const char *source, int source_length, // Check the next token is identifier and matches "key" rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -2997,7 +2997,7 @@ int vec0_parse_vector_column(const char *source, int source_length, // starts with an identifier rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_EMPTY; } @@ -3026,13 +3026,13 @@ int vec0_parse_vector_column(const char *source, int source_length, // left '[' bracket rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_LBRACKET) { + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_LBRACKET) { return SQLITE_EMPTY; } // digit, for vector dimension length rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_DIGIT) { + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_DIGIT) { return SQLITE_ERROR; } dimensions = atoi(token.start); @@ -3042,7 +3042,7 @@ int vec0_parse_vector_column(const char *source, int source_length, // // right ']' bracket rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_RBRACKET) { + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_RBRACKET) { return SQLITE_ERROR; } @@ -3055,7 +3055,7 @@ int vec0_parse_vector_column(const char *source, int source_length, break; } - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_ERROR; } @@ -3070,13 +3070,13 @@ int vec0_parse_vector_column(const char *source, int source_length, } // ensure equal sign after distance_metric rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_EQ) { + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_EQ) { return SQLITE_ERROR; } // distance_metric value, an identifier (L2, cosine, etc) rc = vec0_scanner_next(&scanner, &token); - if (rc != VEC0_TOKEN_RESULT_SOME && + if (rc != VEC0_TOKEN_RESULT_SOME || token.token_type != TOKEN_TYPE_IDENTIFIER) { return SQLITE_ERROR; } diff --git a/tests/test-loadable.py b/tests/test-loadable.py index 0044144b..c8909864 100644 --- a/tests/test-loadable.py +++ b/tests/test-loadable.py @@ -2183,6 +2183,18 @@ def test_vec0_distance_metric(): ] +def test_vec0_column_option_malformed_value(): + # vec0_parse_vector_column's "option value" guard used to be: + # if (rc != VEC0_TOKEN_RESULT_SOME && token.token_type != TOKEN_TYPE_IDENTIFIER) + # With && instead of ||, hitting EOF right after "distance_metric=" left + # `token` uninitialised and the guard could spuriously not fire, falling + # through to use token.start/token.end (uninitialised) as the option value. + # Fixed to || so any non-identifier token (including EOF, where rc != + # VEC0_TOKEN_RESULT_SOME) deterministically rejects the column definition. + with pytest.raises(sqlite3.DatabaseError): + db.execute("create virtual table t using vec0(a float[2] distance_metric=)") + + def test_vec0_vacuum(): db = connect(EXT_PATH) db.execute("create virtual table vec_t using vec0(a float[1]);")