From 78f82caadf185e48e2d73d966ba6c70465907faa Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 5 Aug 2026 10:38:43 +0200 Subject: [PATCH] sqlite: clear SQLTagStore bindings SQLTagStore reused cached statements without clearing values bound by a previous execution. Clear bindings before binding new values and reject parameters that do not correspond to template substitutions. Signed-off-by: Matteo Collina --- src/node_sqlite.cc | 79 +++++++++++------------ src/node_sqlite.h | 4 ++ test/parallel/test-sqlite-template-tag.js | 37 +++++++++++ 3 files changed, 78 insertions(+), 42 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 038af9812f9e..555b635ad391 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3409,6 +3409,35 @@ void SQLTagStore::SizeGetter(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(static_cast(store->sql_tags_.Size())); } +bool SQLTagStore::ResetAndBindStatement( + Environment* env, + StatementSync* stmt, + const FunctionCallbackInfo& args) { + Isolate* isolate = env->isolate(); + int r = stmt->ResetStatement(); + CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); + + r = sqlite3_clear_bindings(stmt->statement_); + CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); + + uint32_t n_params = args.Length() - 1; + int param_count = sqlite3_bind_parameter_count(stmt->statement_); + if (param_count != static_cast(n_params)) { + THROW_ERR_INVALID_ARG_VALUE( + env, + "SQLite parameters must be bound using template literal placeholders."); + return false; + } + + for (int i = 0; i < param_count; ++i) { + if (!stmt->BindValue(args[i + 1], i + 1)) { + return false; + } + } + + return true; +} + void SQLTagStore::Run(const FunctionCallbackInfo& args) { SQLTagStore* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.This()); @@ -3423,15 +3452,8 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { return; } - uint32_t n_params = args.Length() - 1; - int r = stmt->ResetStatement(); - CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); - for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { - Local value = args[i + 1]; - if (!stmt->BindValue(value, i + 1)) { - return; - } + if (!ResetAndBindStatement(env, stmt.get(), args)) { + return; } Local result; @@ -3456,15 +3478,8 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo& args) { return; } - uint32_t n_params = args.Length() - 1; - int r = stmt->ResetStatement(); - CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); - for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { - Local value = args[i + 1]; - if (!stmt->BindValue(value, i + 1)) { - return; - } + if (!ResetAndBindStatement(env, stmt.get(), args)) { + return; } BaseObjectPtr iter = StatementExecutionHelper::Iterate( @@ -3491,18 +3506,8 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { return; } - uint32_t n_params = args.Length() - 1; - Isolate* isolate = env->isolate(); - - int r = stmt->ResetStatement(); - CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); - - int param_count = sqlite3_bind_parameter_count(stmt->statement_); - for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { - Local value = args[i + 1]; - if (!stmt->BindValue(value, i + 1)) { - return; - } + if (!ResetAndBindStatement(env, stmt.get(), args)) { + return; } Local result; @@ -3530,18 +3535,8 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { return; } - uint32_t n_params = args.Length() - 1; - Isolate* isolate = env->isolate(); - - int r = stmt->ResetStatement(); - CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); - - int param_count = sqlite3_bind_parameter_count(stmt->statement_); - for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { - Local value = args[i + 1]; - if (!stmt->BindValue(value, i + 1)) { - return; - } + if (!ResetAndBindStatement(env, stmt.get(), args)) { + return; } auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); }); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 9046b022eea4..956c13ce461d 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -401,6 +401,10 @@ class SQLTagStore : public BaseObject { private: static BaseObjectPtr PrepareStatement( const v8::FunctionCallbackInfo& args); + static bool ResetAndBindStatement( + Environment* env, + StatementSync* stmt, + const v8::FunctionCallbackInfo& args); BaseObjectWeakPtr database_; LRUCache> sql_tags_; friend class StatementExecutionHelper; diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 1a55148cb7b4..494f2d50f76f 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -90,6 +90,43 @@ test('queries with no results', () => { assert.strictEqual(count, 0); }); +test('rejects parameters outside of template expressions', () => { + const ldb = new DatabaseSync(':memory:'); + const lsql = ldb.createTagStore(); + ldb.exec(` + CREATE TABLE secrets(owner TEXT, token TEXT); + INSERT INTO secrets VALUES ('victim', 'secret'); + CREATE TABLE transfers(from_user TEXT, amount INTEGER); + `); + + const expectedError = { + code: 'ERR_INVALID_ARG_VALUE', + message: /must be bound using template literal placeholders/, + }; + + for (const method of ['get', 'all', 'iterate']) { + // Prime the cached statement with a bound value before each attempt. + // eslint-disable-next-line no-unused-expressions + lsql.all`SELECT token FROM secrets WHERE owner = ${'victim'}`; + assert.throws(() => { + // eslint-disable-next-line no-unused-expressions + lsql[method]`SELECT token FROM secrets WHERE owner = ?`; + }, expectedError); + } + + // eslint-disable-next-line no-unused-expressions + lsql.run`INSERT INTO transfers VALUES (${'victim'},${100})`; + assert.throws(() => { + // eslint-disable-next-line no-unused-expressions + lsql.run`INSERT INTO transfers VALUES (?,?)`; + }, expectedError); + assert.strictEqual( + ldb.prepare('SELECT COUNT(*) AS count FROM transfers').get().count, + 1); + + ldb.close(); +}); + test('TagStore capacity, size, and clear', () => { assert.strictEqual(sql.capacity, 10); assert.strictEqual(sql.size, 0);