Skip to content

Conversation

TonyMasson
Copy link
Contributor

🚨 Critical Memory Safety Fix

Problem

The Transaction API had a use-after-free vulnerability where Transaction stored a raw *mut CBLDatabase pointer without ensuring the underlying Database remains alive.

Dangerous scenario:

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 CBLDatabaseTransaction.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

Testing

  • ✅ Existing tests pass (no functional change)
  • ✅ Memory safety guaranteed by Rust ownership
  • ✅ Clippy and formatting checks pass

Risk Assessment

  • High priority fix - prevents undefined behavior
  • No API breaking changes - same public interface
  • No performance impact - reference counting is lightweight

🤖 Generated with Claude Code

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 <[email protected]>
@TonyMasson TonyMasson requested a review from a team as a code owner July 18, 2025 08:00
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
@Nao-ris
Copy link
Contributor

Nao-ris commented Aug 19, 2025

Pas sûr que ça compile côté billeo-engine, l'objet Database pourrait ne pas être transférable entre les threads. A tester.

@TonyMasson TonyMasson marked this pull request as draft August 26, 2025 15:03
@TonyMasson
Copy link
Contributor Author

A réouvrir plus tard

@TonyMasson TonyMasson closed this Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants