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

Closed
mceachen wants to merge 3 commits into from
+444 −14
Closed

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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");

@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.

The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:

db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row

Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.

The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.

db->FinalizeStatements();
db->DeleteSessions();
int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
if (statement_ == nullptr) {
return;
}
sqlite3_finalize(statement_);
statement_ = nullptr;
InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {

@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.

main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.

Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.

Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.

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 DownExpand 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
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

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

 Closed

mceachen
wants to merge
3
commits into

from


+444

−14

 Closed


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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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");
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't cover the authorizer callback, and that leaves the original use-after-free reachable.
The authorizer runs during statement preparation, and preparation can happen inside sqlite3_step: on SQLITE_SCHEMA the public sqlite3_step wrapper calls sqlite3Reprepare in a loop (deps/sqlite/sqlite3.c:94612-94614), which reaches sqlite3AuthCheck. So:
db.setAuthorizer(()=>{db.close();return0;});db.function('f',()=>{db.exec('CREATE TABLE z (a)');return1;});db.prepare('SELECT f(v) FROM t').all();// multi-row
Row 1's f() does DDL, which expires all prepared statements. On row 2 sqlite3_step returns SQLITE_SCHEMA, reprepares in place, and invokes the authorizer — where IsInUserFunctionCallback() is false, so db.close() goes through. FinalizeStatements() then finalizes the very VM whose sqlite3_step frame is live (crash 1 from the description), and zeroes connection_. Through StatementSync::Run the follow-on sqlite3_last_insert_rowid(db->Connection()) derefs null (crash 2) — commit 2 dropped the connection-null check commit 1 added.
The commit message defers the authorizer to a separate change, but this isn't a coverage nicety: it's the same crash, still reachable from pure JS, with the interim mitigations removed. Note main already wraps AuthorizerCallback (src/node_sqlite.cc:2552) — see my other comment.
 db->FinalizeStatements();
 db->DeleteSessions();
 int r = sqlite3_close_v2(db->connection_);
Expand DownExpand 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 DownExpand Up@@ -2573,6 +2588,9 @@ StatementSync::~StatementSync() {
}

void StatementSync::Finalize() {
 if (statement_ == nullptr) {
 return;
 }
 sqlite3_finalize(statement_);
 statement_ = nullptr;
 InvalidateColumnNameCache();
Expand DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 DownExpand 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 numberDiff line numberDiff 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() {
@TrevorBurnhamTrevorBurnhamAug 5, 2026

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on bbf51ad24cb (2026-05-08) and adds a second mechanism alongside one main already has, with narrower coverage.
main carries IsInCallback() / callback_depth_ / class CallbackDepthGuard (src/node_sqlite.h:232-234,249,409-416) applied at five sites — including AuthorizerCallback (node_sqlite.cc:2552) and the sqlite3changeset_apply call (:2409) — plus post-callback IsOpen() checks in xFunc/xStepBase/xValueBase/GetAggregate.
Concretely, the renamed message here fails an existing test: test/parallel/test-sqlite-udf-close.js:33 on main asserts the exact string 'database cannot be closed while in a callback' for all of all/get/run/iterate.
Worth rebasing and building on the existing guard rather than in parallel with it. Two things not to lose in the process: main's authorizer and changeset coverage, and its post-callback IsOpen() checks. That should also make the unrelated sqlite3_reset → ResetStatement() changes in SQLTagStore disappear, since those already match main.
 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 DownExpand 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