Merge pull request #2663 from weiznich/bugfix/fix_use_after_free

Fix a use-after-free bug in our Sqlite backend

CHANGELOG.md

@@ -4,6 +4,14 @@ All user visible changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/), as described
 for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md)
 
+## [1.4.6] - 2021-03-05
+
+### Fixed
+
+* Fixed a use-after-free issue in the `QueryableByName` implementation
+  of our `Sqlite` backend
+* Updated several dependencies
+
 ## [1.4.5] - 2020-06-09
 
 ### Fixed
@@ -1640,3 +1648,4 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 [1.4.3]: https://github.com/diesel-rs/diesel/compare/v1.4.2...v1.4.3
 [1.4.4]: https://github.com/diesel-rs/diesel/compare/v1.4.3...v1.4.4
 [1.4.5]: https://github.com/diesel-rs/diesel/compare/v1.4.4...v1.4.5
+[1.4.6]: https://github.com/diesel-rs/diesel/compare/v1.4.5...v1.4.6

diesel/Cargo.toml

@@ -16,7 +16,7 @@ byteorder = "1.0"
 diesel_derives = "~1.4.0"
 chrono = { version = "0.4", optional = true }
 libc = { version = "0.2.0", optional = true }
-libsqlite3-sys = { version = ">=0.8.0, <0.19.0", optional = true, features = ["min_sqlite_version_3_7_16"] }
+libsqlite3-sys = { version = ">=0.8.0, <0.21.0", optional = true, features = ["min_sqlite_version_3_7_16"] }
 mysqlclient-sys = { version = ">=0.1.0, <0.3.0", optional = true }
 pq-sys = { version = ">=0.3.0, <0.5.0", optional = true }
 quickcheck = { version = "0.4", optional = true }
@@ -25,7 +25,7 @@ time = { version = "0.1", optional = true }
 url = { version = "1.4.0", optional = true }
 uuid = { version = ">=0.2.0, <0.7.0", optional = true, features = ["use_std"] }
 uuidv07 = { version = ">=0.7.0, <0.9.0", optional = true, package = "uuid"}
-ipnetwork = { version = ">=0.12.2, <0.17.0", optional = true }
+ipnetwork = { version = ">=0.12.2, <0.18.0", optional = true }
 num-bigint = { version = ">=0.1.41, <0.3", optional = true }
 num-traits = { version = ">=0.1.37, <0.3", optional = true }
 num-integer = { version = ">=0.1.33, <0.3", optional = true }

diesel/src/sqlite/connection/statement_iterator.rs

@@ -42,28 +42,39 @@ where
 
 pub struct NamedStatementIterator<'a, T> {
     stmt: StatementUse<'a>,
-    column_indices: HashMap<&'a str, usize>,
+    // The actual lifetime of the stored column name is
+    // not really `'a`, but it's impossible to have a better
+    // fitting lifetime here.
+    // See the `Statement::field_name` method for details
+    // how long the underlying livetime is valid
+    column_indices: Option<HashMap<&'a str, usize>>,
     _marker: PhantomData<T>,
 }
 
 impl<'a, T> NamedStatementIterator<'a, T> {
     #[allow(clippy::new_ret_no_self)]
     pub fn new(stmt: StatementUse<'a>) -> QueryResult<Self> {
-        let column_indices = (0..stmt.num_fields())
+        Ok(NamedStatementIterator {
+            stmt,
+            column_indices: None,
+            _marker: PhantomData,
+        })
+    }
+
+    fn populate_column_indices(&mut self) -> QueryResult<()> {
+        let column_indices = (0..self.stmt.num_fields())
             .filter_map(|i| {
-                stmt.field_name(i).map(|column| {
+                self.stmt.field_name(i).map(|column| {
                     let column = column
                         .to_str()
                         .map_err(|e| DeserializationError(e.into()))?;
                     Ok((column, i))
                 })
             })
             .collect::<QueryResult<_>>()?;
-        Ok(NamedStatementIterator {
-            stmt,
-            column_indices,
-            _marker: PhantomData,
-        })
+
+        self.column_indices = Some(column_indices);
+        Ok(())
     }
 }
 
@@ -78,8 +89,17 @@ where
             Ok(row) => row,
             Err(e) => return Some(Err(e)),
         };
+        if self.column_indices.is_none() {
+            if let Err(e) = self.populate_column_indices() {
+                return Some(Err(e));
+            }
+        }
         row.map(|row| {
-            let row = row.into_named(&self.column_indices);
+            let row = row.into_named(
+                self.column_indices
+                    .as_ref()
+                    .expect("it's there because we populated it above"),
+            );
             T::build(&row).map_err(DeserializationError)
         })
     }

diesel/src/sqlite/connection/stmt.rs

@@ -58,9 +58,16 @@ impl Statement {
         unsafe { ffi::sqlite3_column_count(self.inner_statement.as_ptr()) as usize }
     }
 
-    /// The lifetime of the returned CStr is shorter than self. This function
-    /// should be tied to a lifetime that ends before the next call to `reset`
-    unsafe fn field_name<'a>(&self, idx: usize) -> Option<&'a CStr> {
+    /// The lifetime of the returned CStr is shorter than self.
+    ///
+    /// > The returned string pointer is valid until either the prepared statement
+    /// > is destroyed by sqlite3_finalize() or until the statement is automatically
+    /// > reprepared by the first call to sqlite3_step() for a particular
+    /// > run or until the next call to sqlite3_column_name()
+    /// > or sqlite3_column_name16() on the same column.
+    ///
+    /// https://www.sqlite.org/c3ref/column_name.html
+    unsafe fn field_name<'a, 'b: 'a>(&'a self, idx: usize) -> Option<&'b CStr> {
         let ptr = ffi::sqlite3_column_name(self.inner_statement.as_ptr(), idx as libc::c_int);
         if ptr.is_null() {
             None

diesel_cli/Cargo.toml

@@ -27,7 +27,7 @@ tempfile = "3.0.0"
 toml = "0.4.6"
 url = { version = "1.4.0", optional = true }
 barrel = { version = ">= 0.5.0", optional = true, features = ["diesel"] }
-libsqlite3-sys = { version = ">=0.8.0, <0.19.0", optional = true, features = ["min_sqlite_version_3_7_16"] }
+libsqlite3-sys = { version = ">=0.8.0, <0.21.0", optional = true, features = ["min_sqlite_version_3_7_16"] }
 
 [dev-dependencies]
 difference = "1.0"

diesel_tests/Cargo.toml

@@ -21,8 +21,8 @@ dotenv = ">=0.8, <0.11"
 quickcheck = { version = "0.4", features = ["unstable"] }
 uuid = { version = ">=0.7.0, <0.9.0" }
 serde_json = { version=">=0.9, <2.0" }
-ipnetwork = ">=0.12.2, <0.17.0"
-bigdecimal = ">= 0.0.10, <= 0.1.0"
+ipnetwork = ">=0.12.2, <0.18.0"
+bigdecimal = ">= 0.0.10, < 0.2.0"
 
 [features]
 default = []