sqlite: fix crash on db.close() from inside a user function

Calling db.close() from inside a user-defined function callback while
sqlite3_step is on the call stack caused two distinct crashes:

1. DatabaseSync::Close ran sqlite3_finalize on the statement whose
   sqlite3_step frame was still active, freeing the VM that step was
   executing. The outer step then operated on freed memory.

2. Even if (1) is avoided, StatementExecutionHelper::Run dereferenced
   db->Connection() via sqlite3_last_insert_rowid / sqlite3_changes64
   after step returned. The reentrant close zeroed connection_, so
   the deref crashed.

Add a MarkStepping() RAII guard wrapped around every sqlite3_step
caller. If Finalize() is called while stepping_, defer it; the
guard's destructor runs the deferred finalize after step returns.
Add a connection-null check in StatementExecutionHelper::Run before
the connection-dependent reads, throwing ERR_INVALID_STATE.

Fixes: #63180
Signed-off-by: Matthew McEachen <matthew@photostructure.com>

Author: mceachen

src/node_sqlite.cc

@@ -2563,6 +2563,13 @@ StatementSync::~StatementSync() {
 }
 
 void StatementSync::Finalize() {
+  if (statement_ == nullptr) {
+    return;
+  }
+  if (stepping_) {
+    finalize_pending_ = true;
+    return;
+  }
   sqlite3_finalize(statement_);
   statement_ = nullptr;
   InvalidateColumnNameCache();
@@ -2888,6 +2895,10 @@ MaybeLocal<Object> StatementExecutionHelper::Run(Environment* env,
   EscapableHandleScope scope(isolate);
   sqlite3_step(stmt);
   int r = sqlite3_reset(stmt);
+  if (db->Connection() == nullptr) {
+    THROW_ERR_INVALID_STATE(env, "database connection was closed mid-query");
+    return MaybeLocal<Object>();
+  }
   CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, MaybeLocal<Object>());
 
   sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection());
@@ -3016,9 +3027,9 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
-
   Local<Value> result;
+  auto step = stmt->MarkStepping();
+  auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
   if (StatementExecutionHelper::All(env,
                                     stmt->db_.get(),
                                     stmt->statement_,
@@ -3066,6 +3077,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Value> result;
+  auto step = stmt->MarkStepping();
   if (StatementExecutionHelper::Get(env,
                                     stmt->db_.get(),
                                     stmt->statement_,
@@ -3090,6 +3102,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Object> result;
+  auto step = stmt->MarkStepping();
   if (StatementExecutionHelper::Run(
           env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
           .ToLocal(&result)) {
@@ -3354,6 +3367,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Object> result;
+  auto step = stmt->MarkStepping();
   if (StatementExecutionHelper::Run(
           env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
           .ToLocal(&result)) {
@@ -3425,6 +3439,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Value> result;
+  auto step = stmt->MarkStepping();
   if (StatementExecutionHelper::Get(env,
                                     stmt->db_.get(),
                                     stmt->statement_,
@@ -3463,8 +3478,9 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
     }
   }
 
-  auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
   Local<Value> result;
+  auto step = stmt->MarkStepping();
+  auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
   if (StatementExecutionHelper::All(env,
                                     stmt->db_.get(),
                                     stmt->statement_,
@@ -3691,6 +3707,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
       iter->statement_reset_generation_ != iter->stmt_->reset_generation_,
       "iterator was invalidated");
 
+  auto step = iter->stmt_->MarkStepping();
   int r = sqlite3_step(iter->stmt_->statement_);
   if (r != SQLITE_ROW) {
     CHECK_ERROR_OR_THROW(

src/node_sqlite.h

@@ -284,6 +284,19 @@ class StatementSync : public BaseObject {
   void Finalize();
   bool IsFinalized();
 
+  // RAII guard: defer Finalize() if called while sqlite3_step is on the
+  // stack for this statement. Wrap every sqlite3_step caller.
+  inline auto MarkStepping() {
+    stepping_ = true;
+    return OnScopeLeave([this]() {
+      stepping_ = false;
+      if (finalize_pending_) {
+        finalize_pending_ = false;
+        Finalize();
+      }
+    });
+  }
+
   SET_MEMORY_INFO_NAME(StatementSync)
   SET_SELF_SIZE(StatementSync)
 
@@ -295,6 +308,8 @@ class StatementSync : public BaseObject {
   bool use_big_ints_;
   bool allow_bare_named_params_;
   bool allow_unknown_named_params_;
+  bool stepping_ = false;
+  bool finalize_pending_ = false;
   uint64_t reset_generation_ = 0;
   std::optional<std::map<std::string, std::string>> bare_named_params_;
   inline int ResetStatement();

test/parallel/test-sqlite-custom-functions.js

@@ -411,4 +411,88 @@ suite('DatabaseSync.prototype.function()', () => {
       message: /database is not open/,
     });
   });
+
+  suite('reentrant db.close() from inside a user function callback', () => {
+    function setup() {
+      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();
+      db.prepare('INSERT INTO t VALUES (2, 20)').run();
+      db.function('close_db', (x) => {
+        try { db.close(); } catch { /* already closed on later rows */ }
+        return x;
+      });
+      return db;
+    }
+
+    test('.all() completes without crashing', () => {
+      const db = setup();
+      const rows = db.prepare('SELECT close_db(v) AS v FROM t').all();
+      assert.ok(Array.isArray(rows));
+    });
+
+    test('.get() completes without crashing', () => {
+      const db = setup();
+      const row = db.prepare('SELECT close_db(v) AS v FROM t').get();
+      assert.deepStrictEqual(row, { __proto__: null, v: 10 });
+    });
+
+    test('.run() throws ERR_INVALID_STATE rather than crashing', () => {
+      const db = new DatabaseSync(':memory:');
+      db.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, v INTEGER)');
+      db.function('close_db', (x) => {
+        try { db.close(); } catch { /* */ }
+        return x;
+      });
+      assert.throws(
+        () => db.prepare('INSERT INTO t SELECT close_db(1), close_db(2)').run(),
+        { code: 'ERR_INVALID_STATE', message: /closed mid-query/ },
+      );
+    });
+
+    test('iterator returns first row, then throws on next()', () => {
+      const db = setup();
+      const iter = db.prepare('SELECT close_db(v) AS v FROM t').iterate();
+      assert.strictEqual(iter.next().done, false);
+      assert.throws(() => iter.next(), {
+        code: 'ERR_INVALID_STATE',
+        message: /statement has been finalized/,
+      });
+    });
+
+    test('SQL tag store .run() throws ERR_INVALID_STATE', () => {
+      const db = new DatabaseSync(':memory:');
+      db.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, v INTEGER)');
+      db.function('close_db', (x) => {
+        try { db.close(); } catch { /* */ }
+        return x;
+      });
+      const sql = db.createTagStore(4);
+      assert.throws(
+        () => sql.run`INSERT INTO t SELECT close_db(${1}), close_db(${2})`,
+        { code: 'ERR_INVALID_STATE', message: /closed mid-query/ },
+      );
+    });
+
+    test('SQL tag store .all() completes without crashing', () => {
+      const db = setup();
+      const sql = db.createTagStore(4);
+      const rows = sql.all`SELECT close_db(v) AS v FROM t`;
+      assert.ok(Array.isArray(rows));
+    });
+
+    test('SQL tag store .get() completes without crashing', () => {
+      const db = setup();
+      const sql = db.createTagStore(4);
+      const row = sql.get`SELECT close_db(v) AS v FROM t`;
+      assert.deepStrictEqual(row, { __proto__: null, v: 10 });
+    });
+
+    test('a fresh database works after the reentrant close', () => {
+      setup().prepare('SELECT close_db(v) FROM t').all();
+      const db2 = new DatabaseSync(':memory:');
+      assert.deepStrictEqual(db2.prepare('SELECT 1 AS x').get(),
+                             { __proto__: null, x: 1 });
+    });
+  });
 });