From e33b0cdc2b3c5785901363991c811174353979ca Mon Sep 17 00:00:00 2001 From: Trevor Burnham Date: Sat, 8 Aug 2026 19:56:16 -0400 Subject: [PATCH] sqlite: reject connection access from authorizer callbacks SQLite requires that an authorizer callback not modify the connection that invoked it, and counts sqlite3_prepare_v2() and sqlite3_step() as modifications. node:sqlite let an authorizer callback call prepare(), exec(), the statement execution methods, and other connection-mutating APIs on the same DatabaseSync. Track authorizer depth on DatabaseSync with an RAII guard around the callback, and throw ERR_INVALID_STATE from the affected entry points while the callback is on the stack. The depth is per-connection, so other connections stay usable from the callback. The guard covers every authorizer invocation, not just those from an explicit prepare(), since SQLite may re-prepare a statement during sqlite3_step() after a schema change. serialize() and the session changeset() and patchset() methods prepare statements internally, so they re-enter the authorizer too. Reentry through changeset() does not terminate: it recurses until the process is killed, with no way to catch it from JavaScript. Reentering a statement that is currently being stepped is a separate hazard, and a memory-safety one rather than a contract violation. Finalizing it frees the virtual machine that sqlite3_step() is running, and re-running it through run(), get(), all(), iterate(), or the equivalent tag store methods resets that virtual machine mid-execution. Both crash. Any callback SQLite invokes during execution can reach them, not only an authorizer, so a user-defined function is enough. Track the statements currently being stepped and reject reentry into those, which leaves a user-defined function free to prepare, run, and finalize its own helper statements. Tracking is a stack so that nested execution is handled, and covers the paired sqlite3_reset() calls, which can run JavaScript through an aggregate's xFinal. Disposal stays idempotent, since throwing for an already-finalized statement would demote a `using` scope's exception to a SuppressedError. Signed-off-by: Trevor Burnham Fixes: https://github.com/nodejs/node/issues/63207 Assisted-by: claude:opus-5 --- doc/api/sqlite.md | 25 +++ src/node_sqlite.cc | 93 +++++++++- src/node_sqlite.h | 49 +++++ test/parallel/test-sqlite-authz.js | 247 ++++++++++++++++++++++++- test/parallel/test-sqlite-udf-close.js | 156 ++++++++++++++++ 5 files changed, 566 insertions(+), 4 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index ae194ff2acaf..e30842d7b804 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -439,6 +439,11 @@ wrapper around [`sqlite3_create_function_v2()`][]. * `callback` {Function|null} The authorizer function to set, or `null` to @@ -464,6 +469,26 @@ The callback must return one of the following constants: * `SQLITE_DENY` - Deny the operation (causes an error). * `SQLITE_IGNORE` - Ignore the operation (silently skip). +SQLite requires that the authorizer callback not modify the database connection +that invoked it, which includes preparing and stepping statements. Methods that +would do so throw an error with code `ERR_INVALID_STATE` while the callback is +on the stack, including `database.prepare()`, `database.exec()`, the execution +methods of that connection's statements, iterators, and tag stores, and +`database.setAuthorizer()` itself. Other connections remain usable. + +The callback can also be invoked from within `statement.run()`, +`statement.get()`, and similar methods, because SQLite may re-prepare a +statement during execution after a schema change. + +Separately, a statement that is currently being executed cannot be reentered. +Calling `statement.close()` on it would free the virtual machine that is +running, and re-running it through `statement.run()`, `statement.get()`, +`statement.all()`, `statement.iterate()`, or the equivalent tag store methods +would reset that virtual machine mid-execution. All of these throw an +`ERR_INVALID_STATE` error instead. This applies to any callback SQLite invokes +during execution, such as a user-defined function. Other statements on the +connection remain usable. + ```cjs const { DatabaseSync, constants } = require('node:sqlite'); const db = new DatabaseSync(':memory:'); diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 0898e450a503..efe683cf8731 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -96,6 +96,27 @@ inline MaybeLocal Utf8StringMaybeOneByte(Isolate* isolate, } \ } while (0) +// SQLite requires that an authorizer callback not modify the connection that +// invoked it. Preparing and stepping statements both count as modifying it. +// See https://www.sqlite.org/c3ref/set_authorizer.html. +#define THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db) \ + THROW_AND_RETURN_ON_BAD_STATE( \ + (env), \ + (db)->IsInAuthorizerCallback(), \ + "database cannot be accessed from an authorizer callback") + +// A statement's virtual machine cannot be reentered while sqlite3_step() is +// running it. Finalizing it frees the VM outright, and re-running it resets the +// VM mid-execution; both are use-after-free rather than merely a contract +// violation. Callbacks that SQLite invokes during execution are therefore +// barred from reaching the statement being stepped, though other statements on +// the connection stay usable. +#define THROW_AND_RETURN_IF_STEPPING(env, stmt) \ + THROW_AND_RETURN_ON_BAD_STATE( \ + (env), \ + (stmt)->db_->IsSteppingStatement((stmt)->statement_), \ + "statement is already being executed") + #define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \ do { \ switch (sqlite3_##from##_type(__VA_ARGS__)) { \ @@ -825,6 +846,12 @@ Intercepted DatabaseSyncLimits::LimitsSetter( return Intercepted::kYes; } + if (limits->database_->IsInAuthorizerCallback()) { + THROW_ERR_INVALID_STATE( + env, "database cannot be accessed from an authorizer callback"); + return Intercepted::kYes; + } + if (!value->IsNumber()) { THROW_ERR_INVALID_ARG_TYPE( isolate, "Limit value must be a non-negative integer or Infinity."); @@ -1081,6 +1108,7 @@ void DatabaseSync::CreateTagStore(const FunctionCallbackInfo& args) { THROW_ERR_INVALID_STATE(env, "database is not open"); return; } + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); int capacity = 1000; if (args.Length() > 0 && !args[0]->IsUndefined()) { if (!args[0]->IsNumber()) { @@ -1483,6 +1511,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -1606,6 +1635,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -1630,6 +1660,7 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -1803,6 +1834,7 @@ void DatabaseSync::Serialize(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); std::string db_name = "main"; if (!args[0]->IsUndefined()) { @@ -1937,6 +1969,7 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); Utf8Value name(env->isolate(), args[0].As()); Local options = args[1].As(); Local start_v; @@ -2148,6 +2181,7 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { DatabaseSync* db; ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); sqlite3_session* pSession; int r = sqlite3session_create(db->connection_, db_name.c_str(), &pSession); @@ -2317,6 +2351,7 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); if (!args[0]->IsUint8Array()) { THROW_ERR_INVALID_ARG_TYPE( @@ -2452,6 +2487,7 @@ void DatabaseSync::EnableLoadExtension( ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); Isolate* isolate = env->isolate(); if (!args[0]->IsBoolean()) { @@ -2480,6 +2516,7 @@ void DatabaseSync::EnableDefensive(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); Isolate* isolate = env->isolate(); if (!args[0]->IsBoolean()) { @@ -2505,6 +2542,7 @@ void DatabaseSync::LoadExtension(const FunctionCallbackInfo& args) { env, !db->allow_load_extension_, "extension loading is not allowed"); THROW_AND_RETURN_ON_BAD_STATE( env, !db->enable_load_extension_, "extension loading is not allowed"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -2533,6 +2571,7 @@ void DatabaseSync::SetAuthorizer(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); Isolate* isolate = env->isolate(); @@ -2569,6 +2608,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data, const char* param4) { DatabaseSync* db = static_cast(user_data); CallbackDepthGuard guard(db); + AuthorizerDepthGuard authorizer_guard(db); Environment* env = db->env(); Isolate* isolate = env->isolate(); HandleScope handle_scope(isolate); @@ -2682,12 +2722,20 @@ void StatementSync::Close(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_STEPPING(env, stmt); stmt->Close(); } void StatementSync::Dispose(const FunctionCallbackInfo& args) { StatementSync* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + // Disposal is idempotent, so an already-finalized statement is a no-op even + // inside a callback. + if (stmt->IsFinalized()) { + return; + } + THROW_AND_RETURN_IF_STEPPING(env, stmt); stmt->Close(); } @@ -2968,6 +3016,7 @@ MaybeLocal StatementExecutionHelper::All(Environment* env, LocalVector row_values(isolate); LocalVector row_keys(isolate); + SteppingStatementGuard stepping(db, stmt); while ((r = sqlite3_step(stmt)) == SQLITE_ROW) { if (num_cols == 0) { num_cols = sqlite3_column_count(stmt); @@ -3010,8 +3059,14 @@ MaybeLocal StatementExecutionHelper::Run(Environment* env, bool use_big_ints) { Isolate* isolate = env->isolate(); EscapableHandleScope scope(isolate); - sqlite3_step(stmt); - int r = sqlite3_reset(stmt); + int r; + { + // sqlite3_reset() can run JavaScript through an aggregate's xFinal, so it + // stays inside the guard. + SteppingStatementGuard stepping(db, stmt); + sqlite3_step(stmt); + r = sqlite3_reset(stmt); + } CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, MaybeLocal()); sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection()); @@ -3086,6 +3141,9 @@ MaybeLocal StatementExecutionHelper::Get(Environment* env, bool use_big_ints) { Isolate* isolate = env->isolate(); EscapableHandleScope scope(isolate); + // Declared before the reset below so that it outlives it: sqlite3_reset() + // can run JavaScript through an aggregate's xFinal. + SteppingStatementGuard stepping(db, stmt); auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt); }); int r = sqlite3_step(stmt); @@ -3132,6 +3190,8 @@ void StatementSync::All(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get()); + THROW_AND_RETURN_IF_STEPPING(env, stmt); Isolate* isolate = env->isolate(); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); @@ -3159,6 +3219,8 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get()); + THROW_AND_RETURN_IF_STEPPING(env, stmt); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3182,6 +3244,8 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get()); + THROW_AND_RETURN_IF_STEPPING(env, stmt); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3206,6 +3270,8 @@ void StatementSync::Run(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get()); + THROW_AND_RETURN_IF_STEPPING(env, stmt); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3488,6 +3554,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, !session->database_->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get()); BaseObjectPtr stmt = PrepareStatement(args); @@ -3495,6 +3562,8 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_IF_STEPPING(env, stmt.get()); + if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3514,6 +3583,7 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, !session->database_->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get()); BaseObjectPtr stmt = PrepareStatement(args); @@ -3521,6 +3591,8 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_IF_STEPPING(env, stmt.get()); + if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3542,6 +3614,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, !session->database_->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get()); BaseObjectPtr stmt = PrepareStatement(args); @@ -3549,6 +3622,8 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_IF_STEPPING(env, stmt.get()); + if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3571,6 +3646,7 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, !session->database_->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get()); BaseObjectPtr stmt = PrepareStatement(args); @@ -3578,6 +3654,8 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_IF_STEPPING(env, stmt.get()); + if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3597,6 +3675,10 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { void SQLTagStore::Clear(const FunctionCallbackInfo& args) { SQLTagStore* store; ASSIGN_OR_RETURN_UNWRAP(&store, args.This()); + Environment* env = Environment::GetCurrent(args); + if (store->database_) { + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, store->database_.get()); + } store->sql_tags_.Clear(); } @@ -3790,6 +3872,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, iter->stmt_->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, iter->stmt_->db_.get()); Isolate* isolate = env->isolate(); auto iter_template = getLazyIterTemplate(env); @@ -3812,6 +3895,10 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { iter->statement_reset_generation_ != iter->stmt_->reset_generation_, "iterator was invalidated"); + // sqlite3_reset() can run JavaScript through an aggregate's xFinal, so it + // stays inside the guard. + SteppingStatementGuard stepping(iter->stmt_->db_.get(), + iter->stmt_->statement_); int r = sqlite3_step(iter->stmt_->statement_); if (r != SQLITE_ROW) { CHECK_ERROR_OR_THROW( @@ -3867,6 +3954,7 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, iter->stmt_->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, iter->stmt_->db_.get()); Isolate* isolate = env->isolate(); sqlite3_reset(iter->stmt_->statement_); @@ -3945,6 +4033,7 @@ void Session::Changeset(const FunctionCallbackInfo& args) { env, !session->database_->IsOpen(), "database is not open"); THROW_AND_RETURN_ON_BAD_STATE( env, session->session_ == nullptr, "session is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get()); int nChangeset; void* pChangeset; diff --git a/src/node_sqlite.h b/src/node_sqlite.h index b4446e5db859..2a87f073b4b4 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -9,6 +9,7 @@ #include "sqlite3.h" #include "util.h" +#include #include #include #include @@ -233,6 +234,26 @@ class DatabaseSync : public BaseObject { void DecrementCallbackDepth() { --callback_depth_; } bool IsInCallback() const { return callback_depth_ > 0; } + // SQLite forbids an authorizer callback from doing anything that modifies + // the database connection that invoked it, which includes preparing and + // stepping statements. See https://www.sqlite.org/c3ref/set_authorizer.html. + void IncrementAuthorizerDepth() { ++authorizer_depth_; } + void DecrementAuthorizerDepth() { --authorizer_depth_; } + bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; } + + // Finalizing a statement frees its virtual machine, so a callback that + // SQLite invokes from inside sqlite3_step() must not finalize the statement + // being stepped. Other statements on the connection are safe to finalize. + void PushSteppingStatement(sqlite3_stmt* stmt) { + stepping_statements_.push_back(stmt); + } + void PopSteppingStatement() { stepping_statements_.pop_back(); } + bool IsSteppingStatement(sqlite3_stmt* stmt) const { + return std::find(stepping_statements_.begin(), + stepping_statements_.end(), + stmt) != stepping_statements_.end(); + } + SET_MEMORY_INFO_NAME(DatabaseSync) SET_SELF_SIZE(DatabaseSync) @@ -247,6 +268,8 @@ class DatabaseSync : public BaseObject { sqlite3* connection_; bool ignore_next_sqlite_error_; int callback_depth_ = 0; + int authorizer_depth_ = 0; + std::vector stepping_statements_; std::set backups_; std::unordered_set sessions_; @@ -426,6 +449,32 @@ class CallbackDepthGuard { DatabaseSync* db_; }; +class SteppingStatementGuard { + public: + SteppingStatementGuard(DatabaseSync* db, sqlite3_stmt* stmt) : db_(db) { + db_->PushSteppingStatement(stmt); + } + ~SteppingStatementGuard() { db_->PopSteppingStatement(); } + SteppingStatementGuard(const SteppingStatementGuard&) = delete; + SteppingStatementGuard& operator=(const SteppingStatementGuard&) = delete; + + private: + DatabaseSync* db_; +}; + +class AuthorizerDepthGuard { + public: + explicit AuthorizerDepthGuard(DatabaseSync* db) : db_(db) { + db_->IncrementAuthorizerDepth(); + } + ~AuthorizerDepthGuard() { db_->DecrementAuthorizerDepth(); } + AuthorizerDepthGuard(const AuthorizerDepthGuard&) = delete; + AuthorizerDepthGuard& operator=(const AuthorizerDepthGuard&) = delete; + + private: + DatabaseSync* db_; +}; + class UserDefinedFunction { public: UserDefinedFunction(Environment* env, diff --git a/test/parallel/test-sqlite-authz.js b/test/parallel/test-sqlite-authz.js index 69c075a57e2e..6ae2447bde7f 100644 --- a/test/parallel/test-sqlite-authz.js +++ b/test/parallel/test-sqlite-authz.js @@ -1,7 +1,7 @@ 'use strict'; -const { skipIfSQLiteMissing } = require('../common'); -skipIfSQLiteMissing(); +const common = require('../common'); +common.skipIfSQLiteMissing(); const assert = require('node:assert'); const { DatabaseSync, constants } = require('node:sqlite'); @@ -288,3 +288,246 @@ suite('DatabaseSync.prototype.setAuthorizer()', () => { }); }); }); + +// SQLite forbids an authorizer callback from modifying the connection that +// invoked it, which includes preparing and stepping statements. +// See https://www.sqlite.org/c3ref/set_authorizer.html. +suite('authorizer callback reentrancy', () => { + const expectedError = 'ERR_INVALID_STATE: database cannot be accessed ' + + 'from an authorizer callback'; + const steppingError = + 'ERR_INVALID_STATE: statement is already being executed'; + + // Calls each of `cases` from inside an authorizer callback, and returns a + // `name -> outcome` map of what each one threw. + const runInAuthorizer = (db, cases) => { + const outcomes = {}; + for (const [name, fn] of Object.entries(cases)) { + let ran = false; + db.setAuthorizer(() => { + if (!ran) { + ran = true; + try { + fn(); + outcomes[name] = 'did not throw'; + } catch (err) { + outcomes[name] = `${err.code}: ${err.message}`; + } + } + return constants.SQLITE_OK; + }); + db.exec('SELECT 1'); + db.setAuthorizer(null); + if (!ran) { + outcomes[name] = 'authorizer callback did not run'; + } + } + return outcomes; + }; + + // Builds the expected `name -> outcome` map for the given case names. + const allRejected = (cases) => Object.fromEntries( + Object.keys(cases).map((name) => [name, expectedError]), + ); + + it('rejects database methods', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + const cases = { + prepare: () => db.prepare('SELECT 1'), + exec: () => db.exec('SELECT 1'), + setAuthorizer: () => db.setAuthorizer(null), + createSession: () => db.createSession(), + applyChangeset: () => db.applyChangeset(new Uint8Array([1])), + createTagStore: () => db.createTagStore(), + serialize: () => db.serialize(), + function: () => db.function('noop', () => 1), + aggregate: () => db.aggregate('agg', { start: 0, step: (acc) => acc }), + enableLoadExtension: () => db.enableLoadExtension(false), + limits: () => { db.limits.length = 100; }, + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + }); + + // close() and deserialize() tear down the connection, so the pre-existing + // callback depth guard already rejects them with its own message. + it('rejects methods the callback depth guard already covers', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + const snapshot = db.serialize(); + const cases = { + close: () => db.close(), + deserialize: () => db.deserialize(snapshot), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), { + close: 'ERR_INVALID_STATE: database cannot be closed while in a callback', + deserialize: 'ERR_INVALID_STATE: database cannot be deserialized ' + + 'while in a callback', + }); + }); + + it('rejects statement methods', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const stmt = db.prepare('SELECT x FROM t'); + const cases = { + run: () => stmt.run(), + get: () => stmt.get(), + all: () => stmt.all(), + iterate: () => stmt.iterate(), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + }); + + // Only the statement being stepped is unsafe to finalize. Other statements + // on the connection have their own virtual machines, so finalizing them from + // a callback is allowed. + it('allows finalizing a statement that is not being executed', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const closeStmt = db.prepare('SELECT x FROM t'); + const disposeStmt = db.prepare('SELECT x FROM t'); + const cases = { + close: () => closeStmt.close(), + dispose: () => disposeStmt[Symbol.dispose](), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), { + close: 'did not throw', + dispose: 'did not throw', + }); + }); + + // Disposal is idempotent, so a statement that is already finalized must stay + // a no-op even inside a callback. Throwing here would turn a `using` scope's + // real exception into a SuppressedError. + it('allows disposing an already-finalized statement', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + const stmt = db.prepare('SELECT x FROM t'); + stmt.close(); + const cases = { dispose: () => stmt[Symbol.dispose]() }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), { + dispose: 'did not throw', + }); + }); + + it('rejects session changeset methods', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER PRIMARY KEY, y TEXT)'); + const session = db.createSession({ table: 't' }); + db.exec("INSERT INTO t VALUES (1, 'a')"); + const cases = { + changeset: () => session.changeset(), + patchset: () => session.patchset(), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + }); + + // A statement being re-prepared inside sqlite3_step() is the case that + // actually crashes, because that statement's VM is mid-execution. + it('rejects finalizing the statement being stepped', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const stmt = db.prepare('SELECT x FROM t'); + stmt.get(); + db.exec('ALTER TABLE t ADD COLUMN y INTEGER'); + + let outcome = 'authorizer callback did not run'; + let ran = false; + db.setAuthorizer(() => { + if (!ran) { + ran = true; + try { + stmt.close(); + outcome = 'did not throw'; + } catch (err) { + outcome = `${err.code}: ${err.message}`; + } + } + return constants.SQLITE_OK; + }); + + stmt.get(); + + assert.strictEqual(outcome, steppingError); + }); + + it('rejects iterator methods', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const iter = db.prepare('SELECT x FROM t').iterate(); + const cases = { + next: () => iter.next(), + return: () => iter.return(), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + iter.return(); + }); + + it('rejects tag store methods', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + const sql = db.createTagStore(10); + const cases = { + run: () => sql.run`SELECT 1`, + get: () => sql.get`SELECT 1`, + all: () => sql.all`SELECT 1`, + iterate: () => sql.iterate`SELECT 1`, + clear: () => sql.clear(), + }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + }); + + // A statement may be re-prepared during sqlite3_step() after a schema + // change, which invokes the authorizer without an explicit prepare() call. + it('rejects reentry when the authorizer runs during a re-prepare', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const stmt = db.prepare('SELECT x FROM t'); + stmt.get(); + db.exec('ALTER TABLE t ADD COLUMN y INTEGER'); + + let outcome = 'authorizer callback did not run'; + let ran = false; + db.setAuthorizer(() => { + if (!ran) { + ran = true; + try { + db.prepare('SELECT 1'); + outcome = 'did not throw'; + } catch (err) { + outcome = `${err.code}: ${err.message}`; + } + } + return constants.SQLITE_OK; + }); + + stmt.get(); + + assert.strictEqual(outcome, expectedError); + }); + + it('allows access again after the authorizer returns', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (x INTEGER)'); + const cases = { prepare: () => db.prepare('SELECT 1') }; + + assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases)); + + db.setAuthorizer(() => constants.SQLITE_OK); + assert.deepStrictEqual(db.prepare('SELECT 1 AS v').get(), { __proto__: null, v: 1 }); + }); +}); diff --git a/test/parallel/test-sqlite-udf-close.js b/test/parallel/test-sqlite-udf-close.js index 86794029b457..c3cb331d6bcd 100644 --- a/test/parallel/test-sqlite-udf-close.js +++ b/test/parallel/test-sqlite-udf-close.js @@ -36,4 +36,160 @@ for (const method of ['all', 'get', 'run', 'iterate']) { assert.strictEqual(db.isOpen, true); db.close(); }); + + // Finalizing the statement being stepped frees the virtual machine that + // sqlite3_step() is still running, so this must throw rather than crash. + test(`statement.close() from a UDF during statement.${method}()`, () => { + const db = new DatabaseSync(':memory:'); + db.exec(` + CREATE TABLE data (value INTEGER); + INSERT INTO data VALUES (1), (2), (3); + `); + + let statement; + db.function('close_stmt', (value) => { + statement.close(); + return value; + }); + + statement = db.prepare('SELECT close_stmt(value) FROM data'); + assert.throws(() => { + if (method === 'iterate') { + for (const row of statement.iterate()) { + assert.ok(row); + } + } else { + statement[method](); + } + }, { + code: 'ERR_INVALID_STATE', + message: 'statement is already being executed', + }); + + db.close(); + }); + + // Re-running the statement being stepped resets its virtual machine + // mid-execution, which is the same use-after-free as finalizing it. + for (const reentrant of ['run', 'get', 'all', 'iterate']) { + test(`statement.${reentrant}() from a UDF during ` + + `statement.${method}()`, () => { + const db = new DatabaseSync(':memory:'); + db.exec(` + CREATE TABLE data (value INTEGER, padding TEXT); + INSERT INTO data VALUES (1, '${'x'.repeat(400)}'), + (2, '${'y'.repeat(400)}'), + (3, '${'z'.repeat(400)}'); + `); + + let statement; + let thrown; + db.function('reenter', (value) => { + if (thrown === undefined) { + try { + statement[reentrant](); + thrown = null; + } catch (err) { + thrown = err; + } + } + return value; + }); + + statement = db.prepare('SELECT reenter(value), padding FROM data'); + if (method === 'iterate') { + for (const row of statement.iterate()) { + assert.ok(row); + } + } else { + statement[method](); + } + + assert.ok(thrown, `${reentrant}() was not rejected`); + assert.strictEqual(thrown.code, 'ERR_INVALID_STATE'); + assert.strictEqual(thrown.message, 'statement is already being executed'); + + db.close(); + }); + } + + // Tag store methods resolve to a cached statement, which may be the one + // currently being stepped. + test(`tag store reentry during statement.${method}()`, () => { + const db = new DatabaseSync(':memory:'); + const sql = db.createTagStore(10); + db.exec(` + CREATE TABLE data (value INTEGER, padding TEXT); + INSERT INTO data VALUES (1, '${'x'.repeat(400)}'), + (2, '${'y'.repeat(400)}'); + `); + + let thrown; + db.function('reenter_tag', (value) => { + if (thrown === undefined) { + try { + // The identical tagged literal resolves to the same cached + // statement that is mid-execution. + // eslint-disable-next-line no-unused-expressions + sql.run`SELECT reenter_tag(value), padding FROM data`; + thrown = null; + } catch (err) { + thrown = err; + } + } + return value; + }); + + if (method === 'iterate') { + for (const row of sql.iterate`SELECT reenter_tag(value), padding FROM data`) { + assert.ok(row); + } + } else { + // eslint-disable-next-line no-unused-expressions + sql[method]`SELECT reenter_tag(value), padding FROM data`; + } + + assert.ok(thrown, 'tag store reentry was not rejected'); + assert.strictEqual(thrown.code, 'ERR_INVALID_STATE'); + assert.strictEqual(thrown.message, 'statement is already being executed'); + + db.close(); + }); + + // A UDF may prepare and finalize its own helper statements. Only the + // statement being stepped is off limits. + test(`UDF finalizes its own statement during statement.${method}()`, () => { + const db = new DatabaseSync(':memory:'); + db.exec(` + CREATE TABLE data (value INTEGER); + INSERT INTO data VALUES (1), (2), (3); + CREATE TABLE lookup (key INTEGER, label TEXT); + INSERT INTO lookup VALUES (1, 'one'), (2, 'two'), (3, 'three'); + `); + + db.function('lookup_label', (value) => { + const helper = db.prepare('SELECT label FROM lookup WHERE key = ?'); + const label = helper.get(value).label; + helper.close(); + return label; + }); + + const statement = db.prepare('SELECT lookup_label(value) AS l FROM data'); + if (method === 'iterate') { + const labels = []; + for (const row of statement.iterate()) { + labels.push(row.l); + } + assert.deepStrictEqual(labels, ['one', 'two', 'three']); + } else if (method === 'all') { + assert.deepStrictEqual(statement.all().map((r) => r.l), + ['one', 'two', 'three']); + } else if (method === 'get') { + assert.strictEqual(statement.get().l, 'one'); + } else { + statement.run(); + } + + db.close(); + }); }