fix EvalStateBuilder.set_lookup_path results in use after free

nixide/src/expr/eval_state_builder.rs

@@ -1,5 +1,7 @@
 use std::cell::RefCell;
-use std::ffi::{CString, c_char};
+use std::ffi::CString;
+use std::hint;
+use std::iter;
 use std::ptr::{self, NonNull};
 use std::rc::Rc;
 
@@ -7,10 +9,10 @@ use super::EvalState;
 #[cfg(feature = "flakes")]
 use crate::FlakeSettings;
 use crate::Store;
-use crate::errors::{ErrorContext, NixideResult};
+use crate::errors::{ErrorContext, NixideResult, ToNixideResult as _};
 use crate::sys;
+use crate::util::wrap;
 use crate::util::wrappers::AsInnerPtr;
-use crate::util::{panic_issue_call_failed, wrap};
 
 /// Builder for Nix evaluation state.
 ///
@@ -104,47 +106,38 @@ impl EvalStateBuilder {
     }
 
     pub fn set_lookup_path<P: AsRef<str>>(self, paths: Vec<P>) -> NixideResult<Self> {
-        // let paths_len = paths.len();
+        // WARNING: Creating a c-style array requires leaking each path
-        // let paths_capacity = paths.capacity();
+        // WARNING: then appending a null terminator. Otherwise Rust
-
+        // WARNING: will free the paths and leave dangling pointers.
-        // XXX: TODO: use the `AsCArray` trait instead
+        let (c_paths, paths_len, paths_cap) = paths
-        let mut ptrs: Vec<*const c_char> = paths
             .into_iter()
-            .map(|p| {
+            .map(|p| -> NixideResult<_> {
                 CString::new(p.as_ref())
-                    .unwrap_or_else(|err| {
+                    .to_nixide_result()
-                        panic_issue_call_failed!(
+                    .map(|s| s.into_raw().cast_const())
-                            "given string {} contains a NUL byte ({})",
-                            p.as_ref(),
-                            err
-                        )
             })
-                    .into_raw() as *const c_char
+            .chain(iter::once(Ok(ptr::null())))
-            })
+            .collect::<NixideResult<Vec<_>>>()?
-            .collect();
+            .into_raw_parts();
 
-        ptrs.push(ptr::null());
+        let builder = wrap::nix_fn!(|ctx: &ErrorContext| unsafe {
+            sys::nix_eval_state_builder_set_lookup_path(ctx.as_ptr(), self.as_ptr(), c_paths);
+            self
+        })?;
 
-        // Leak the Vec and return as mutable pointer
+        // WARNING: We must reconstruct the leaked elements manually.
-        let ptr = ptrs.as_mut_ptr();
+        // WARNING: Using the `black_box` compiler hint ensures this
-        std::mem::forget(ptrs);
+        // WARNING: block is never compiled away.
+        unsafe {
+            hint::black_box(
+                Vec::from_raw_parts(c_paths, paths_len, paths_cap)
+                    .into_iter()
+                    .map(|p| hint::black_box(CString::from_raw(p.cast_mut())))
+                    .collect::<Vec<_>>(),
+            );
+        }
 
-        let result = wrap::nix_fn!(|ctx: &ErrorContext| unsafe {
+        Ok(builder)
-            sys::nix_eval_state_builder_set_lookup_path(ctx.as_ptr(), self.as_ptr(), ptr);
-        })
-        .map(|()| self);
-
-        // ensure all allocated memory is dropped
-        // XXX: TODO!!
-        // unsafe {
-        //     Vec::from_raw_parts(ptr, paths_len, paths_capacity)
-        //         .into_iter()
-        //         .map(|p| {
-        //             _ = CString::from_raw(p as *mut c_char);
-        //         })
-        // };
-
-        result
     }
 }
 

nixide/src/store/path.rs

@@ -6,7 +6,7 @@ use std::rc::Rc;
 
 use super::Store;
 use crate::NixideResult;
-use crate::errors::{ErrorContext, new_nixide_error};
+use crate::errors::{ErrorContext, ToNixideResult as _, new_nixide_error};
 use crate::stdext::CCharPtrExt as _;
 use crate::sys;
 use crate::util::panic_issue_call_failed;
@@ -85,7 +85,7 @@ impl StorePath {
     /// ```
     ///
     pub fn new(store: Rc<RefCell<Store>>, path: &str) -> NixideResult<Self> {
-        let c_path = CString::new(path).or(Err(new_nixide_error!(StringNulByte)))?;
+        let c_path = CString::new(path).to_nixide_result()?;
 
         let inner = wrap::nix_ptr_fn!(|ctx: &ErrorContext| unsafe {
             sys::nix_store_parse_path(ctx.as_ptr(), store.borrow().as_ptr(), c_path.as_ptr())