Skip to content

Commit 2a217a9

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 e53d87a commit 2a217a9

5 files changed

Lines changed: 659 additions & 5 deletions

File tree

doc/api/sqlite.md

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

440440
<!-- YAML
441441
added: v24.10.0
442+
changes:
443+
- version: REPLACEME
444+
pr-url: https://github.com/nodejs/node/pull/65156
445+
description: Accessing the invoking database connection from the authorizer
446+
callback now throws.
442447
-->
443448

444449
* `callback` {Function|null} The authorizer function to set, or `null` to
@@ -464,6 +469,31 @@ The callback must return one of the following constants:
464469
* `SQLITE_DENY` - Deny the operation (causes an error).
465470
* `SQLITE_IGNORE` - Ignore the operation (silently skip).
466471

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

0 commit comments

Comments
 (0)