sqlite: reject busy statement finalization in authorizer · nodejs/node@fce6754 · GitHub
Skip to content

Commit fce6754

Browse files
trivikraduh95
authored andcommitted
sqlite: reject busy statement finalization in authorizer
An iterator can leave a statement active between sqlite3_step() calls. Finalizing that statement from an authorizer callback can release its locks and change the outcome of the statement being authorized. Reject close() and Symbol.dispose when the statement is busy and the connection is in an authorizer callback. Continue allowing idle statements to be finalized. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: #65369 Fixes: #65368 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 29f010f commit fce6754

4 files changed

Lines changed: 53 additions & 8 deletions

File tree

doc/api/sqlite.md

Lines changed: 1 addition & 1 deletion

src/node_sqlite.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
160160
(db)->IsInAuthorizerCallback(), \
161161
"database cannot be accessed from an authorizer callback")
162162

163+
// Finalizing a busy statement from an authorizer can release its locks and
164+
// change the outer statement's outcome.
165+
#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
166+
THROW_AND_RETURN_ON_BAD_STATE( \
167+
(env), \
168+
(stmt)->db_->IsInAuthorizerCallback() && \
169+
sqlite3_stmt_busy((stmt)->statement_.get()), \
170+
"database cannot be accessed from an authorizer callback")
171+
163172
// A statement's virtual machine cannot be reentered while sqlite3_step() is
164173
// running it. Finalizing it frees the VM outright, and re-running it resets the
165174
// VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
29002909
THROW_AND_RETURN_ON_BAD_STATE(
29012910
env, stmt->IsFinalized(), "statement has been finalized");
29022911
THROW_AND_RETURN_IF_STEPPING(env, stmt);
2912+
THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
29032913
stmt->Close();
29042914
}
29052915

@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
29132923
return;
29142924
}
29152925
THROW_AND_RETURN_IF_STEPPING(env, stmt);
2926+
THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
29162927
stmt->Close();
29172928
}
29182929

src/node_sqlite.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
299299
void DecrementAuthorizerDepth() { --authorizer_depth_; }
300300
bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
301301

302-
// Finalizing a statement frees its virtual machine, so a callback that
303-
// SQLite invokes from inside sqlite3_step() must not finalize the statement
304-
// being stepped. Other statements on the connection are safe to finalize.
302+
// A callback must not finalize the statement being stepped. Other statements
303+
// are safe unless they are busy during an authorizer callback.
305304
void PushSteppingStatement(sqlite3_stmt* stmt) {
306305
stepping_statements_.push_back(stmt);
307306
}

test/parallel/test-sqlite-authz.js

Lines changed: 39 additions & 4 deletions

0 commit comments

Comments
 (0)