sqlite: reject closing a session from a callback · nodejs/node@b5f0558 · GitHub
Skip to content

Commit b5f0558

Browse files
TrevorBurnhamaduh95
authored andcommitted
sqlite: reject closing a session from a callback
SQLite runs "PRAGMA table_xinfo" from inside its pre-update hook, while it is still walking the connection's session list. Deleting a session from a callback that the PRAGMA triggers frees memory that walk is still using. Both an authorizer callback and a 'sqlite.db.query' subscriber reach that window, and either one segfaults. Reject session.close() and Symbol.dispose when the connection is inside any callback. An already-closed session stays a no-op so that disposal remains idempotent. Fixes: #65428 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65454 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 0650dca commit b5f0558

3 files changed

Lines changed: 174 additions & 4 deletions

File tree

‎doc/api/sqlite.md‎

Lines changed: 7 additions & 3 deletions

‎src/node_sqlite.cc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,16 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
169169
sqlite3_stmt_busy((stmt)->statement_.get()), \
170170
"database cannot be accessed from an authorizer callback")
171171

172+
// SQLite's session module reaches back into JavaScript from inside the
173+
// pre-update hook, while it is still walking the connection's session list and
174+
// reading the table it found there. Deleting a session frees memory that walk
175+
// is still using, so no callback may close one.
176+
#define THROW_AND_RETURN_IF_SESSION_IN_CALLBACK(env, session) \
177+
THROW_AND_RETURN_ON_BAD_STATE( \
178+
(env), \
179+
(session)->database_->IsInCallback(), \
180+
"session cannot be closed while in a callback")
181+
172182
// A statement's virtual machine cannot be reentered while sqlite3_step() is
173183
// running it. Finalizing it frees the VM outright, and re-running it resets the
174184
// VM mid-execution; both are use-after-free rather than merely a contract
@@ -4358,6 +4368,9 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) {
43584368
env, session->session_ == nullptr, "session is not open");
43594369
THROW_AND_RETURN_ON_BAD_STATE(
43604370
env, session->is_generating_changeset_, "session is currently in use");
4371+
// Checked last: changeset generation runs the authorizer, so both conditions
4372+
// hold in that case and the more specific message above has to win.
4373+
THROW_AND_RETURN_IF_SESSION_IN_CALLBACK(env, session);
43614374

43624375
session->Delete();
43634376
}
@@ -4371,6 +4384,7 @@ void Session::Dispose(const FunctionCallbackInfo<Value>& args) {
43714384
}
43724385
THROW_AND_RETURN_ON_BAD_STATE(
43734386
env, session->is_generating_changeset_, "session is currently in use");
4387+
THROW_AND_RETURN_IF_SESSION_IN_CALLBACK(env, session);
43744388

43754389
session->Delete();
43764390
}

‎test/parallel/test-sqlite-session.js‎

Lines changed: 153 additions & 1 deletion

0 commit comments

Comments
 (0)