From 318bdd3d1960c5b8bd898253b2169bc1dfe463fe Mon Sep 17 00:00:00 2001 From: _cry64 Date: Tue, 14 Apr 2026 10:10:13 +1000 Subject: [PATCH] fix EvalStateBuilder.set_lookup_path results in use after free --- nixide/src/expr/eval_state_builder.rs | 69 ++++++++++++--------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/nixide/src/expr/eval_state_builder.rs b/nixide/src/expr/eval_state_builder.rs index 117cf8a..6933de7 100644 --- a/nixide/src/expr/eval_state_builder.rs +++ b/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>(self, paths: Vec

) -> NixideResult { - // let paths_len = paths.len(); - // let paths_capacity = paths.capacity(); - - // XXX: TODO: use the `AsCArray` trait instead - let mut ptrs: Vec<*const c_char> = paths + // WARNING: Creating a c-style array requires leaking each path + // WARNING: then appending a null terminator. Otherwise Rust + // WARNING: will free the paths and leave dangling pointers. + let (c_paths, paths_len, paths_cap) = paths .into_iter() - .map(|p| { + .map(|p| -> NixideResult<_> { CString::new(p.as_ref()) - .unwrap_or_else(|err| { - panic_issue_call_failed!( - "given string {} contains a NUL byte ({})", - p.as_ref(), - err - ) - }) - .into_raw() as *const c_char + .to_nixide_result() + .map(|s| s.into_raw().cast_const()) }) - .collect(); + .chain(iter::once(Ok(ptr::null()))) + .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 - let ptr = ptrs.as_mut_ptr(); - std::mem::forget(ptrs); + // WARNING: We must reconstruct the leaked elements manually. + // WARNING: Using the `black_box` compiler hint ensures this + // 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::>(), + ); + } - let result = wrap::nix_fn!(|ctx: &ErrorContext| unsafe { - 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 + Ok(builder) } }