Repository: diesel-rs/diesel (language: rust)

Issue:
Unsoundness with `diesel::sqlite::SerializedDatabase::new()`
`diesel::sqlite::SerializedDatabase` looks to be memory-unsafe.

## Setup

### Versions

- **Rust:** 1.79.0
- **Diesel:** 2.2.1
- **Database:** sqlite
- **Operating System** macOS Sonoma 14.5

### Feature Flags

- **diesel:** sqlite

## Problem Description

[`diesel::sqlite::SerializedDatabase::new()`](https://docs.diesel.rs/2.2.x/src/diesel/sqlite/connection/serialized_database.rs.html#16) is a safe function that takes a pointer.  [`SerializedDatabase::drop()`](https://docs.diesel.rs/2.2.x/src/diesel/sqlite/connection/serialized_database.rs.html#39) always passes that pointer to [`sqlite3_free()`](https://www.sqlite.org/c3ref/free.html).  That means that the following call will cause heap corruption:
```rust
diesel::sqlite::SerializedDatabase::new(&mut 0, 1);
```

### What are you trying to accomplish?

Safely serialize and deserialize a SQLite database. 

### What is the expected output?

### What is the actual output?

### Are you seeing any additional errors?

### Steps to reproduce

`cargo run` segfaults when I put the above example into `main()`:
```sh
$ cat Cargo.toml
[package]
name = "diesel-unsoundness"
[dependencies]
diesel = { version = "2.2.1", default-features = false, features = ["sqlite"] }
$ cat src/main.rs
fn main() {
    diesel::sqlite::SerializedDatabase::new(&mut 0, 1);
}
$ cargo run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/diesel-unsoundness`
zsh: segmentation fault  cargo run
```

## Checklist

- [x] I have already looked over the [issue tracker](https://github.com/diesel-rs/diesel/issues) and the [discussion forum](https://github.com/diesel-rs/diesel/discussions) for similar possible closed issues.
<!--
If you are unsure if your issue is a duplicate of an existing issue please link the issue in question here
--> 
- [x] This issue can be reproduced on Rust's stable channel. (Your issue will be
  closed if this is not the case)
- [x] This issue can be reproduced without requiring a third party crate

<!--
Thank you for your submission!  You're helping make Diesel more robust 🎉

We'll try to respond as quickly as possible.
-->



Discussion / hints:
Thanks for opening filling this issue. This shouldn't happen. From a look at the source code it seems like the `diesel::sqlite::SerializedDatabase::new` is not supposed to be public at all. (It should be marked as unsafe nevertheless).

Given that semver allows us to perform "breaking changes" to fix serve bugs and I would consider that a serve bug I propose to:

* Mark this function as `pub(crate)` instead of `pub`
* Mark it as unsafe and fix the internal code correspondingly. 

cc @diesel-rs/contributors 
There likely should be some way for the end user to create a `SerializedDatabase` since it’s needed to subsequently deserialize a DB (which is SQLite’s supported method of repopulating a DB without using the filesystem), but I certainly agree with the priority of fixing the issue first.

(My guess would be that a proper solution would be to have the initializer create the underlying buffer with `sqlite3_malloc()` and provide a mechanism to populate it with a Reader, Writer, or similar.)
We don't use `SerializedDatabase` directly for deserialization. The relevant function accepts a `&[u8]` instead. https://docs.diesel.rs/2.2.x/diesel/sqlite/struct.SqliteConnection.html#method.deserialize_readonly_database_from_buffer

The `SerializedDatabase` type mostly exists to have a RAII wrapper around the pointer returned by the serialize function. It just implement `Deref<Target = [u8]>` for to allow the user to access the actual data + handles the deallocation. 

Fix patch:
diff --git a/diesel/src/sqlite/connection/serialized_database.rs b/diesel/src/sqlite/connection/serialized_database.rs
index 8584d9109b09..28a98a286131 100644
--- a/diesel/src/sqlite/connection/serialized_database.rs
+++ b/diesel/src/sqlite/connection/serialized_database.rs
@@ -13,7 +13,10 @@ pub struct SerializedDatabase {
 
 impl SerializedDatabase {
     /// Creates a new `SerializedDatabase` with the given data pointer and length.
-    pub fn new(data: *mut u8, len: usize) -> Self {
+    ///
+    /// SAFETY: The data pointer needs to be returned by sqlite
+    ///         and the length must match the underlying buffer pointer
+    pub(crate) unsafe fn new(data: *mut u8, len: usize) -> Self {
         Self { data, len }
     }
 


Test patch: