Fix use-after-free in exqlite_interrupt: add interrupt_mutex; NULL guard execute

exqlite_interrupt() read conn->db and called sqlite3_interrupt() without any
lock, while a concurrent close() could sqlite3_close_v2() and NULL conn->db
between the read and the call → use-after-free / segfault.

The connection lock cannot be used here: running queries hold it for their
full duration, and interrupt() must not block waiting for them to finish.

Add a dedicated interrupt_mutex to connection_t.  close() acquires it after
sqlite3_close_v2() sets conn->db = NULL, and interrupt() acquires it before
reading conn->db.  This eliminates the TOCTOU entirely.

Also add a conn->db == NULL guard inside the lock in exqlite_execute so that
a racing call to execute after close returns an error rather than crashing.

Author: mjc

c_src/sqlite3_nif.c

@@ -52,6 +52,7 @@ typedef struct connection
 {
     sqlite3* db;
     ErlNifMutex* mutex;
+    ErlNifMutex* interrupt_mutex;
     ErlNifPid update_hook_pid;
 } connection_t;
 
@@ -336,8 +337,14 @@ exqlite_open(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
         enif_mutex_destroy(mutex);
         return make_error_tuple(env, am_out_of_memory);
     }
-    conn->db    = db;
-    conn->mutex = mutex;
+    conn->db              = db;
+    conn->mutex           = mutex;
+    conn->interrupt_mutex = enif_mutex_create("exqlite:interrupt");
+    if (conn->interrupt_mutex == NULL) {
+        // conn->db and conn->mutex are set; the destructor will clean them up.
+        enif_release_resource(conn);
+        return make_error_tuple(env, am_failed_to_create_mutex);
+    }
 
     result = enif_make_resource(env, conn);
     enif_release_resource(conn);
@@ -398,7 +405,12 @@ exqlite_close(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
         return make_sqlite3_error_tuple(env, rc, conn->db);
     }
 
+    // Acquire interrupt_mutex so any concurrent exqlite_interrupt() finishes
+    // before we NULL out conn->db, eliminating the TOCTOU / use-after-free.
+    enif_mutex_lock(conn->interrupt_mutex);
     conn->db = NULL;
+    enif_mutex_unlock(conn->interrupt_mutex);
+
     connection_release_lock(conn);
 
     return am_ok;
@@ -431,6 +443,11 @@ exqlite_execute(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
 
     connection_acquire_lock(conn);
 
+    if (conn->db == NULL) {
+        connection_release_lock(conn);
+        return make_error_tuple(env, am_connection_closed);
+    }
+
     rc = sqlite3_exec(conn->db, (char*)bin.data, NULL, NULL, NULL);
     if (rc != SQLITE_OK) {
         connection_release_lock(conn);
@@ -1171,6 +1188,11 @@ connection_type_destructor(ErlNifEnv* env, void* arg)
         enif_mutex_destroy(conn->mutex);
         conn->mutex = NULL;
     }
+
+    if (conn->interrupt_mutex) {
+        enif_mutex_destroy(conn->interrupt_mutex);
+        conn->interrupt_mutex = NULL;
+    }
 }
 
 void
@@ -1477,14 +1499,20 @@ exqlite_interrupt(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
         return make_error_tuple(env, am_invalid_connection);
     }
 
-    // DB is already closed, nothing to do here
-    if (conn->db == NULL) {
-        return am_ok;
-    }
-
-    // connection_acquire_lock(conn);
-    sqlite3_interrupt(conn->db);
-    // connection_release_lock(conn);
+    // We deliberately do NOT hold the connection lock here.  A running
+    // query holds the lock for its entire duration; acquiring it in
+    // interrupt() would block until the query finishes, which defeats
+    // the purpose of interrupting it.
+    //
+    // interrupt_mutex is a dedicated lightweight lock shared with close().
+    // close() holds the connection lock (so any running query has already
+    // released it) then acquires interrupt_mutex before nulling conn->db.
+    // interrupt() acquires interrupt_mutex here, so the two cannot overlap.
+    enif_mutex_lock(conn->interrupt_mutex);
+    if (conn->db != NULL) {
+        sqlite3_interrupt(conn->db);
+    }
+    enif_mutex_unlock(conn->interrupt_mutex);
 
     return am_ok;
 }

test/exqlite/sqlite3_test.exs

@@ -659,11 +659,9 @@ defmodule Exqlite.Sqlite3Test do
       :ok = Sqlite3.close(conn)
       :ok = Sqlite3.bind(statement, ["this is a test"])
 
-      {:error, message} =
+      assert {:error, :connection_closed} =
         Sqlite3.execute(conn, "create table test (id integer primary key, stuff text)")
 
-      assert message == "Sqlite3 was invoked incorrectly."
-
       assert :done == Sqlite3.step(conn, statement)
     end
   end
@@ -845,11 +843,10 @@ defmodule Exqlite.Sqlite3Test do
       :ok = Sqlite3.close(conn)
     end
 
-    # Targets the TOCTOU in exqlite_interrupt:
-    # The function checks conn->db == NULL outside the lock, then calls
-    # sqlite3_interrupt(conn->db) without holding the lock.  A concurrent
-    # close() can sqlite3_close_v2() and NULL conn->db between the check and
-    # the call → sqlite3_interrupt on a freed pointer → use-after-free segfault.
+    # Verifies that concurrent interrupt/1 and close/1 are race-free.
+    # exqlite_interrupt acquires interrupt_mutex (shared with close()) before
+    # reading conn->db, and close() acquires it (while holding the conn lock,
+    # after the running query has finished) before nulling conn->db.
     test "concurrent interrupt and close does not segfault" do
       for _ <- 1..500 do
         {:ok, conn} = Sqlite3.open(":memory:")