Skip to content

sqlite: fix crash on db.close() from inside a user function #63183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mceachen wants to merge 3 commits into
base: main
Choose a base branch
from
+444 −14
Open

sqlite: fix crash on db.close() from inside a user function #63183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14cd548
sqlite: fix crash on db.close() from inside a user function
mceachen May 8, 2026
20d7ffa
sqlite: reject forbidden ops inside user-defined function callbacks
mceachen May 9, 2026
c02e2c0
sqlite: narrow user-callback guard to per-statement reentry
mceachen May 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 67 additions & 14 deletions src/node_sqlite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,16 @@ class CustomAggregate {
}

Local<Value> ret;
if (!(self->*mptr)
.Get(isolate)
->Call(env->context(), recv, argc + 1, js_argv.data())
.ToLocal(&ret)) {
self->db_->SetIgnoreNextSQLiteError(true);
sqlite3_result_error(ctx, "", 0);
return;
{
auto guard = self->db_->EnterUserFunctionCallback();
if (!(self->*mptr)
.Get(isolate)
->Call(env->context(), recv, argc + 1, js_argv.data())
.ToLocal(&ret)) {
self->db_->SetIgnoreNextSQLiteError(true);
sqlite3_result_error(ctx, "", 0);
return;
}
}

agg->value.Reset(isolate, ret);
Expand Down Expand Up @@ -422,6 +425,7 @@ class CustomAggregate {
Local<Function>::New(env->isolate(), self->result_fn_);
Local<Value> js_arg[] = {Local<Value>::New(isolate, agg->value)};

auto guard = self->db_->EnterUserFunctionCallback();
if (!fn->Call(env->context(), Null(isolate), 1, js_arg)
.ToLocal(&result)) {
self->db_->SetIgnoreNextSQLiteError(true);
Expand Down Expand Up @@ -455,6 +459,7 @@ class CustomAggregate {
Local<Value> start_v = Local<Value>::New(isolate, start_);
if (start_v->IsFunction()) {
auto fn = start_v.As<Function>();
auto guard = db_->EnterUserFunctionCallback();
MaybeLocal<Value> retval =
fn->Call(env_->context(), Null(isolate), 0, nullptr);
if (!retval.ToLocal(&start_v)) {
Expand Down Expand Up @@ -698,6 +703,7 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
js_argv.emplace_back(local);
}

auto guard = self->db_->EnterUserFunctionCallback();
MaybeLocal<Value> retval =
fn->Call(env->context(), recv, argc, js_argv.data());
Local<Value> result;
Expand Down Expand Up @@ -1420,6 +1426,10 @@ void DatabaseSync::Close(const FunctionCallbackInfo<Value>& 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_ON_BAD_STATE(
env,
db->IsInUserFunctionCallback(),
"database cannot be closed inside a user-defined function callback");
db->FinalizeStatements();
db->DeleteSessions();
int r = sqlite3_close_v2(db->connection_);
Expand Down Expand Up @@ -1808,6 +1818,11 @@ void DatabaseSync::Deserialize(const FunctionCallbackInfo<Value>& 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_ON_BAD_STATE(
env,
db->IsInUserFunctionCallback(),
"database operation is not allowed inside a user-defined function "
"callback");

if (!args[0]->IsUint8Array()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down Expand Up @@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
if (statement_ == nullptr) {
return;
}
sqlite3_finalize(statement_);
statement_ = nullptr;
InvalidateColumnNameCache();
Expand Down Expand Up @@ -3018,6 +3036,8 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");
Isolate* isolate = env->isolate();
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void());
Expand All @@ -3026,9 +3046,9 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
return;
}

auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });

Local<Value> result;
auto step = stmt->MarkStepping();
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
if (StatementExecutionHelper::All(env,
stmt->db_.get(),
stmt->statement_,
Expand All @@ -3045,6 +3065,8 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand All @@ -3068,6 +3090,8 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand All @@ -3076,6 +3100,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
}

Local<Value> result;
auto step = stmt->MarkStepping();
if (StatementExecutionHelper::Get(env,
stmt->db_.get(),
stmt->statement_,
Expand All @@ -3092,6 +3117,8 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand All @@ -3100,6 +3127,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
}

Local<Object> result;
auto step = stmt->MarkStepping();
if (StatementExecutionHelper::Run(
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
.ToLocal(&result)) {
Expand Down Expand Up @@ -3352,8 +3380,11 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {
return;
}

THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");

uint32_t n_params = args.Length() - 1;
int r = sqlite3_reset(stmt->statement_);
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<int>(n_params) && i < param_count; ++i) {
Expand All @@ -3364,6 +3395,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {
}

Local<Object> result;
auto step = stmt->MarkStepping();
if (StatementExecutionHelper::Run(
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
.ToLocal(&result)) {
Expand All @@ -3385,8 +3417,11 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
return;
}

THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");

uint32_t n_params = args.Length() - 1;
int r = sqlite3_reset(stmt->statement_);
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<int>(n_params) && i < param_count; ++i) {
Expand Down Expand Up @@ -3420,10 +3455,13 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
return;
}

THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");

uint32_t n_params = args.Length() - 1;
Isolate* isolate = env->isolate();

int r = sqlite3_reset(stmt->statement_);
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_);
Expand All @@ -3435,6 +3473,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
}

Local<Value> result;
auto step = stmt->MarkStepping();
if (StatementExecutionHelper::Get(env,
stmt->db_.get(),
stmt->statement_,
Expand All @@ -3459,10 +3498,13 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
return;
}

THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsStepping(), "statement is currently being executed");

uint32_t n_params = args.Length() - 1;
Isolate* isolate = env->isolate();

int r = sqlite3_reset(stmt->statement_);
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_);
Expand All @@ -3473,8 +3515,9 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
}
}

auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
Local<Value> result;
auto step = stmt->MarkStepping();
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
if (StatementExecutionHelper::All(env,
stmt->db_.get(),
stmt->statement_,
Expand All @@ -3488,6 +3531,11 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
void SQLTagStore::Clear(const FunctionCallbackInfo<Value>& args) {
SQLTagStore* store;
ASSIGN_OR_RETURN_UNWRAP(&store, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env,
store->database_->IsInUserFunctionCallback(),
"tag store cannot be cleared inside a user-defined function callback");
store->sql_tags_.Clear();
}

Expand Down Expand Up @@ -3679,6 +3727,8 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsStepping(), "statement is currently being executed");
Isolate* isolate = env->isolate();

auto iter_template = getLazyIterTemplate(env);
Expand All @@ -3701,6 +3751,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
iter->statement_reset_generation_ != iter->stmt_->reset_generation_,
"iterator was invalidated");

auto step = iter->stmt_->MarkStepping();
int r = sqlite3_step(iter->stmt_->statement_);
if (r != SQLITE_ROW) {
CHECK_ERROR_OR_THROW(
Expand Down Expand Up @@ -3755,6 +3806,8 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsStepping(), "statement is currently being executed");
Isolate* isolate = env->isolate();

sqlite3_reset(iter->stmt_->statement_);
Expand Down
32 changes: 32 additions & 0 deletions src/node_sqlite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,24 @@ class DatabaseSync : public BaseObject {
}
sqlite3* Connection();

// SQLite forbids closing the database while a user-defined scalar or
// aggregate function callback is on the stack. Wrap every such
// callback with the RAII guard returned by EnterUserFunctionCallback().
// db.close()/deserialize() and SQL tag store .clear() check
// IsInUserFunctionCallback() and refuse to run, since they would
// finalize statements (potentially the running one). Reentry into the
// *running* statement (recursive step, reset, or finalize) is
// detected separately via the per-statement
// StatementSync::IsStepping() flag, which leaves cross-statement use
// (the "lookup" pattern) unaffected.
inline auto EnterUserFunctionCallback() {
user_function_callback_depth_++;
return OnScopeLeave([this]() { user_function_callback_depth_--; });
}
bool IsInUserFunctionCallback() const {
return user_function_callback_depth_ > 0;
}

// In some situations, such as when using custom functions, it is possible
// that SQLite reports an error while JavaScript already has a pending
// exception. In this case, the SQLite error should be ignored. These methods
Expand All @@ -241,6 +259,7 @@ class DatabaseSync : public BaseObject {
bool enable_load_extension_;
sqlite3* connection_;
bool ignore_next_sqlite_error_;
int user_function_callback_depth_ = 0;

std::set<BackupJob*> backups_;
std::set<sqlite3_session*> sessions_;
Expand Down Expand Up @@ -283,6 +302,18 @@ class StatementSync : public BaseObject {
bool GetCachedColumnNames(v8::LocalVector<v8::Name>* keys);
void Finalize();
bool IsFinalized();
bool IsStepping() const { return stepping_; }

// RAII guard: marks this statement as being stepped while alive.
// JS-callable methods that would step, reset, or finalize this
// statement check IsStepping() and throw — that's the
// sqlite3_step / sqlite3_reset / sqlite3_finalize reentry SQLite
// forbids while the statement's user-defined function callback is
// on the stack.
inline auto MarkStepping() {
stepping_ = true;
return OnScopeLeave([this]() { stepping_ = false; });
}

SET_MEMORY_INFO_NAME(StatementSync)
SET_SELF_SIZE(StatementSync)
Expand All @@ -295,6 +326,7 @@ class StatementSync : public BaseObject {
bool use_big_ints_;
bool allow_bare_named_params_;
bool allow_unknown_named_params_;
bool stepping_ = false;
uint64_t reset_generation_ = 0;
std::optional<std::map<std::string, std::string>> bare_named_params_;
inline int ResetStatement();
Expand Down
Loading
Loading