sqlite: prevent database close during callbacks

Co-authored-by: Asroy Cristian Sitorus <asroycristiansitorus@gmail.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64743
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: 2 people

src/node_sqlite.cc

@@ -356,6 +356,7 @@ class CustomAggregate {
                                Global<Function> CustomAggregate::*mptr) {
     CustomAggregate* self =
         static_cast<CustomAggregate*>(sqlite3_user_data(ctx));
+    CallbackDepthGuard guard(self->db_);
     Environment* env = self->env_;
     Isolate* isolate = env->isolate();
     auto agg = self->GetAggregate(ctx);
@@ -402,12 +403,18 @@ class CustomAggregate {
       return;
     }
 
+    if (!self->db_->IsOpen()) {
+      THROW_ERR_INVALID_STATE(env, "database is not open");
+      return;
+    }
+
     agg->value.Reset(isolate, ret);
   }
 
   static inline void xValueBase(sqlite3_context* ctx, bool is_final) {
     CustomAggregate* self =
         static_cast<CustomAggregate*>(sqlite3_user_data(ctx));
+    CallbackDepthGuard guard(self->db_);
     Environment* env = self->env_;
     Isolate* isolate = env->isolate();
     auto agg = self->GetAggregate(ctx);
@@ -433,6 +440,9 @@ class CustomAggregate {
                .ToLocal(&result)) {
         self->db_->SetIgnoreNextSQLiteError(true);
         sqlite3_result_error(ctx, "", 0);
+      } else if (!self->db_->IsOpen()) {
+        THROW_ERR_INVALID_STATE(env, "database is not open");
+        return;
       }
     } else {
       result = Local<Value>::New(isolate, agg->value);
@@ -464,6 +474,10 @@ class CustomAggregate {
         auto fn = start_v.As<Function>();
         MaybeLocal<Value> retval =
             fn->Call(env_->context(), Null(isolate), 0, nullptr);
+        if (!db_->IsOpen()) {
+          THROW_ERR_INVALID_STATE(env_, "database is not open");
+          return nullptr;
+        }
         if (!retval.ToLocal(&start_v)) {
           db_->SetIgnoreNextSQLiteError(true);
           sqlite3_result_error(ctx, "", 0);
@@ -676,6 +690,7 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
                                 sqlite3_value** argv) {
   UserDefinedFunction* self =
       static_cast<UserDefinedFunction*>(sqlite3_user_data(ctx));
+  CallbackDepthGuard guard(self->db_);
   Environment* env = self->env_;
   Isolate* isolate = env->isolate();
   auto recv = Undefined(isolate);
@@ -707,6 +722,12 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
 
   MaybeLocal<Value> retval =
       fn->Call(env->context(), recv, argc, js_argv.data());
+
+  if (!self->db_->IsOpen()) {
+    THROW_ERR_INVALID_STATE(env, "database is not open");
+    return;
+  }
+
   Local<Value> result;
   if (!retval.ToLocal(&result)) {
     // Ignore the SQLite error because a JavaScript exception is pending.
@@ -1453,6 +1474,8 @@ void DatabaseSync::Close(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, db->IsInCallback(), "database cannot be closed while in a callback");
   db->FinalizeStatements();
   db->DeleteSessions();
   int r = sqlite3_close_v2(db->connection_);
@@ -2417,13 +2440,17 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
   BaseObjectPtr<DatabaseSync> guard(db);
 
   ArrayBufferViewContents<uint8_t> buf(args[0]);
-  int r = sqlite3changeset_apply(
-      db->connection_,
-      buf.length(),
-      const_cast<void*>(static_cast<const void*>(buf.data())),
-      context.filterCallback ? xFilter : nullptr,
-      xConflict,
-      static_cast<void*>(&context));
+  int r;
+  {
+    CallbackDepthGuard guard(db);
+    r = sqlite3changeset_apply(
+        db->connection_,
+        buf.length(),
+        const_cast<void*>(static_cast<const void*>(buf.data())),
+        context.filterCallback ? xFilter : nullptr,
+        xConflict,
+        static_cast<void*>(&context));
+  }
   if (r == SQLITE_OK) {
     args.GetReturnValue().Set(true);
     return;
@@ -2558,6 +2585,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data,
                                      const char* param3,
                                      const char* param4) {
   DatabaseSync* db = static_cast<DatabaseSync*>(user_data);
+  CallbackDepthGuard guard(db);
   Environment* env = db->env();
   Isolate* isolate = env->isolate();
   HandleScope handle_scope(isolate);

src/node_sqlite.h

@@ -229,6 +229,10 @@ class DatabaseSync : public BaseObject {
   void SetIgnoreNextSQLiteError(bool ignore);
   bool ShouldIgnoreSQLiteError();
 
+  void IncrementCallbackDepth() { ++callback_depth_; }
+  void DecrementCallbackDepth() { --callback_depth_; }
+  bool IsInCallback() const { return callback_depth_ > 0; }
+
   SET_MEMORY_INFO_NAME(DatabaseSync)
   SET_SELF_SIZE(DatabaseSync)
 
@@ -242,6 +246,7 @@ class DatabaseSync : public BaseObject {
   bool enable_load_extension_;
   sqlite3* connection_;
   bool ignore_next_sqlite_error_;
+  int callback_depth_ = 0;
 
   std::set<BackupJob*> backups_;
   std::unordered_set<Session*> sessions_;
@@ -406,6 +411,19 @@ class SQLTagStore : public BaseObject {
   friend class StatementExecutionHelper;
 };
 
+class CallbackDepthGuard {
+ public:
+  explicit CallbackDepthGuard(DatabaseSync* db) : db_(db) {
+    db_->IncrementCallbackDepth();
+  }
+  ~CallbackDepthGuard() { db_->DecrementCallbackDepth(); }
+  CallbackDepthGuard(const CallbackDepthGuard&) = delete;
+  CallbackDepthGuard& operator=(const CallbackDepthGuard&) = delete;
+
+ private:
+  DatabaseSync* db_;
+};
+
 class UserDefinedFunction {
  public:
   UserDefinedFunction(Environment* env,

test/parallel/test-sqlite-udf-close.js

@@ -0,0 +1,39 @@
+'use strict';
+
+const { skipIfSQLiteMissing } = require('../common');
+skipIfSQLiteMissing();
+const assert = require('node:assert');
+const { test } = require('node:test');
+const { DatabaseSync } = require('node:sqlite');
+
+for (const method of ['all', 'get', 'run', 'iterate']) {
+  test(`database.close() from a UDF during statement.${method}()`, () => {
+    const db = new DatabaseSync(':memory:');
+    db.exec(`
+      CREATE TABLE data (value INTEGER);
+      INSERT INTO data VALUES (1), (2), (3);
+    `);
+
+    db.function('close_db', (value) => {
+      db.close();
+      return value;
+    });
+
+    const statement = db.prepare('SELECT close_db(value) FROM data');
+    assert.throws(() => {
+      if (method === 'iterate') {
+        for (const row of statement.iterate()) {
+          assert.ok(row);
+        }
+      } else {
+        statement[method]();
+      }
+    }, {
+      code: 'ERR_INVALID_STATE',
+      message: 'database cannot be closed while in a callback',
+    });
+
+    assert.strictEqual(db.isOpen, true);
+    db.close();
+  });
+}