Skip to content

Commit ca8ee11

Browse files
committed
sqlite: reject connection access from authorizer callbacks
SQLite requires that an authorizer callback not modify the connection that invoked it, and counts sqlite3_prepare_v2() and sqlite3_step() as modifications. node:sqlite let the callback call prepare(), exec(), the statement execution methods, and other connection-mutating APIs on the same DatabaseSync. Track authorizer depth on DatabaseSync with an RAII guard around the callback and throw ERR_INVALID_STATE from the affected entry points while it is on the stack. Covering every authorizer invocation, including the re-prepare that SQLite can run during sqlite3_step(), exposed a second and distinct hazard: reentering a statement that is currently being stepped is a use-after-free rather than a contract violation, since finalizing it frees the virtual machine under sqlite3_step() and re-running it resets that machine mid-execution. Any callback SQLite invokes during execution can reach it, so a user-defined function is enough. Track the statements currently being stepped and reject reentry into only those, which leaves a user-defined function free to prepare, run, and finalize its own helper statements. Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> Fixes: #63207 Assisted-by: claude:opus-5
1 parent bfa3e98 commit ca8ee11

5 files changed

Lines changed: 661 additions & 7 deletions

File tree

doc/api/sqlite.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,11 @@ wrapper around [`sqlite3_create_function_v2()`][].
442442

443443
<!-- YAML
444444
added: v24.10.0
445+
changes:
446+
- version: REPLACEME
447+
pr-url: https://github.com/nodejs/node/pull/65156
448+
description: Accessing the invoking database connection from the authorizer
449+
callback now throws.
445450
-->
446451

447452
* `callback` {Function|null} The authorizer function to set, or `null` to
@@ -467,6 +472,31 @@ The callback must return one of the following constants:
467472
* `SQLITE_DENY` - Deny the operation (causes an error).
468473
* `SQLITE_IGNORE` - Ignore the operation (silently skip).
469474

475+
SQLite requires that the authorizer callback not modify the database connection
476+
that invoked it, which includes preparing and stepping statements. Methods that
477+
would do so throw an error with code `ERR_INVALID_STATE` while the callback is
478+
on the stack, including `database.prepare()`, `database.exec()`, the execution
479+
methods of that connection's statements, iterators, and tag stores, and
480+
`database.setAuthorizer()` itself. Other connections remain usable.
481+
482+
The callback can also be invoked from within `statement.run()`,
483+
`statement.get()`, and similar methods, because SQLite may re-prepare a
484+
statement during execution after a schema change.
485+
486+
Separately, a statement that is currently being executed cannot be reentered.
487+
Calling `statement.close()` on it would free the virtual machine that is
488+
running, and re-running it through `statement.run()`, `statement.get()`,
489+
`statement.all()`, `statement.iterate()`, `iterator.next()`,
490+
`iterator.return()`, or the equivalent tag store methods would reset that
491+
virtual machine mid-execution. All of these throw an `ERR_INVALID_STATE` error
492+
instead. This applies to any callback SQLite invokes during execution, such as a
493+
user-defined function. Other statements on the connection remain usable.
494+
495+
Operations that touch no SQLite state stay available from the callback:
496+
`sqlTagStore.clear()`, which only drops cached statements, and `next()` and
497+
`return()` on an already-drained iterator, which keep returning
498+
`{ done: true }`.
499+
470500
```cjs
471501
const { DatabaseSync, constants } = require('node:sqlite');
472502
const db = new DatabaseSync(':memory:');

0 commit comments

Comments
 (0)