Skip to content

sqlite: reentrancy into a running statement from a user-defined function is unguarded #65102

Description

@TrevorBurnham

Version

main (verified on 2b350bb8e42)

Platform

Darwin 25.6.0 arm64 (macOS); not platform-specific

Subsystem

sqlite

What steps will reproduce the bug?

#64743 added a database-level callback-depth guard, so db.close() from inside a user-defined function now correctly throws ERR_INVALID_STATE instead of finalizing a statement mid-sqlite3_step() (that was #63180, now fixed). The guard is per-database, so it does not cover a callback reentering the specific statement that invoked it. SQLite forbids calling sqlite3_step() / sqlite3_reset() / sqlite3_finalize() on a statement while that statement's own callback is on the stack.

Two cases:

1. Reentrant iter.next() silently advances the iterator being consumed.

const { DatabaseSync } = require('node:sqlite');
const db = new DatabaseSync(':memory:');
db.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, v INTEGER)');
db.prepare('INSERT INTO t VALUES (1, 10)').run();

let iter;
db.function('reenter', () => {
  iter.next();   // reenters the statement currently being stepped
  return 0;
});

iter = db.prepare('SELECT reenter() FROM t').iterate();
console.log(iter.next());

2. Recursive stmt.get() on the running statement.

const { DatabaseSync } = require('node:sqlite');
const db = new DatabaseSync(':memory:');
let stmt;
db.function('x', () => stmt.get());
stmt = db.prepare('SELECT x()');
stmt.get();

How often does it reproduce? Is there a required condition?

Always. The callback must reenter the same statement object that is currently executing.

What is the expected behavior? Why is that the expected behavior?

Both should throw ERR_INVALID_STATE, consistent with how #64743 handles db.close() from a callback. Cross-statement use from a callback (the common "lookup" pattern — preparing or stepping a different statement) should keep working; only reentry into the running statement is forbidden by the C API.

What do you see instead?

Case 1: no error. The reentrant next() consumes rows from the same VM, so the outer iteration silently skips them:

reentrant next() SUCCEEDED
reentrant next() SUCCEEDED
first: {"done":false,"value":{"reenter()":0}}

Case 2: Maximum call stack size exceeded — a V8 stack overflow rather than a SQLite-level error, so the actual constraint is never reported.

Neither crashes, so this is a correctness issue rather than memory safety.

Additional information

The closed #63183 contained a per-statement IsStepping() RAII flag (MarkStepping() around every sqlite3_step caller, with JS-callable step/reset/finalize checking it) that addresses exactly this, plus regression tests for both cases. That PR was closed in favor of #64743, which covers only the database-level close() path, so the per-statement piece is currently unowned. It may be worth salvaging from that branch.

Activity

  1. TrevorBurnham commented on Aug 9, 2026

    @TrevorBurnham
    ContributorAuthor

    Correction on severity: this is a memory-safety bug, not only a correctness one. Both repros above recurse without bound, so V8's stack overflows before control returns into the corrupted sqlite3_step(), which is why they surface as RangeError. Bounding the reentry to a single call segfaults:

    const { DatabaseSync } = require('node:sqlite');
    const db = new DatabaseSync(':memory:');
    db.exec('CREATE TABLE t (x INTEGER)');
    db.exec('INSERT INTO t VALUES (1), (2), (3)');
    
    let stmt;
    let first = true;
    db.function('f', () => {
      if (first) { first = false; stmt.get(); }
      return 1;
    });
    
    stmt = db.prepare('SELECT f(), x FROM t');
    stmt.all();   // SIGSEGV

    The trigger is sqlite3_reset() on a statement whose own sqlite3_step() frame is live, not the reentrant step. Reentry that resets (get(), run(), all(), and iter.return(), a bare sqlite3_reset) crashes; reentry that only steps (iter.next()) or reads metadata (columns(), sourceSQL, expandedSQL) does not. Reproduces on v24.15.0 and current main.

    The per-statement IsStepping() flag from #63183 therefore needs to gate sqlite3_reset as well, which a database-level depth counter can't express.

  2. TrevorBurnham commented on Aug 14, 2026

    @TrevorBurnham
    ContributorAuthor

    This issue was fixed by #65156.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions