From 9e992acbde971e268addf662bf50c719ede70bef Mon Sep 17 00:00:00 2001
From: TonyMasson <tony.masson@doctolib.com>
Date: Fri, 18 Jul 2025 09:59:45 +0200
Subject: [PATCH 1/2] Fix critical memory safety issue in Transaction API
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PROBLEM:
Transaction was storing a raw *mut CBLDatabase pointer without ensuring
the underlying Database remains alive. This could lead to use-after-free
if the Database was dropped before the Transaction:

```rust
let transaction = {
    let mut db = Database::open("test", None)?;
    db.begin_transaction()?  // db dropped here\!
};
transaction.commit()?; // 💥 Use-after-free\!
```

SOLUTION:
Transaction now owns a Database clone instead of a raw pointer:
- Transaction::new() calls db.clone() which uses reference counting
- Database::clone() calls retain() to increment CBLDatabase ref count
- When Transaction is dropped, Database is automatically released
- CBLDatabase stays alive for the entire Transaction lifetime

CHANGES:
- Transaction.db_ref: *mut CBLDatabase → Transaction.db: Database
- Transaction::new() takes &Database instead of *mut CBLDatabase
- All Transaction methods use self.db.get_ref() instead of raw pointer
- Memory safety now guaranteed through Rust's ownership system

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/database.rs | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/database.rs b/src/database.rs
index 4ffbbf5..0d1947d 100644
--- a/src/database.rs
+++ b/src/database.rs
@@ -359,7 +359,7 @@ impl Database {
     ///
     /// For simpler cases, consider using `in_transaction()` instead.
     pub fn begin_transaction(&mut self) -> Result<Transaction> {
-        Transaction::new(self.get_ref())
+        Transaction::new(self)
     }
 
     /// Encrypts or decrypts a database, or changes its encryption key.
@@ -606,20 +606,21 @@ impl Database {
 /// A database transaction that can be committed or rolled back.
 /// When dropped without being committed, the transaction is automatically rolled back.
 pub struct Transaction {
-    db_ref: *mut CBLDatabase,
+    db: Database,
     committed: bool,
 }
 
 impl Transaction {
-    fn new(db_ref: *mut CBLDatabase) -> Result<Self> {
+    fn new(db: &Database) -> Result<Self> {
+        let db_clone = db.clone();
         unsafe {
             let mut err = CBLError::default();
-            if !CBLDatabase_BeginTransaction(db_ref, &mut err) {
+            if !CBLDatabase_BeginTransaction(db_clone.get_ref(), &mut err) {
                 return failure(err);
             }
         }
         Ok(Transaction {
-            db_ref,
+            db: db_clone,
             committed: false,
         })
     }
@@ -628,7 +629,7 @@ impl Transaction {
     pub fn commit(mut self) -> Result<()> {
         unsafe {
             let mut err = CBLError::default();
-            if !CBLDatabase_EndTransaction(self.db_ref, true, &mut err) {
+            if !CBLDatabase_EndTransaction(self.db.get_ref(), true, &mut err) {
                 return failure(err);
             }
         }
@@ -642,7 +643,7 @@ impl Drop for Transaction {
         if !self.committed {
             unsafe {
                 let mut err = CBLError::default();
-                let _ = CBLDatabase_EndTransaction(self.db_ref, false, &mut err);
+                let _ = CBLDatabase_EndTransaction(self.db.get_ref(), false, &mut err);
             }
         }
     }

From c7f578e3f03d62e0acf48bf53dc965176b8abd94 Mon Sep 17 00:00:00 2001
From: TonyMasson <tony.masson@doctolib.com>
Date: Fri, 18 Jul 2025 10:06:25 +0200
Subject: [PATCH 2/2] Remove unnecessary &mut from begin_transaction()

Transaction::new() only needs &Database (not &mut) because:
- We only call db.clone() which doesn't mutate the Database
- All transaction operations go through the C API via get_ref()
- This makes the API more flexible - no need for mutable Database

This allows usage like:
let db = Database::open("test", None)?;  // immutable
let transaction = db.begin_transaction()?;  // works now
---
 src/database.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/database.rs b/src/database.rs
index 0d1947d..a8d4e7a 100644
--- a/src/database.rs
+++ b/src/database.rs
@@ -358,7 +358,7 @@ impl Database {
     /// it will be rolled back when dropped.
     ///
     /// For simpler cases, consider using `in_transaction()` instead.
-    pub fn begin_transaction(&mut self) -> Result<Transaction> {
+    pub fn begin_transaction(&self) -> Result<Transaction> {
         Transaction::new(self)
     }