From 0819f928a08d4e4cdc60fee728880749465a921d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Mon, 15 Aug 2022 01:37:38 +0200 Subject: [PATCH] Fix potential use after free in MacOS / iOS impl Per Ryan Lopopolo's review: > This bit makes me a bit scared with the `Dropping` type and the `CStr` being dropped while a borrowed `&str` is taken from name. Is the `.map(|name| name.to_owned())` a use after free? > > To be sure, I'd probably restructure all of these combinators to use short circuit return to make sure the `Dropping` and `CStr` wrappers are dropped in the right order. --- src/tz_macos.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/tz_macos.rs b/src/tz_macos.rs index a42c00a..6e831fd 100644 --- a/src/tz_macos.rs +++ b/src/tz_macos.rs @@ -4,20 +4,19 @@ use core_foundation_sys::timezone::{CFTimeZoneCopySystem, CFTimeZoneGetName}; pub(crate) fn get_timezone_inner() -> Result { unsafe { - Dropping::new(CFTimeZoneCopySystem()) - .and_then(|tz| Dropping::new(CFTimeZoneGetName(tz.0))) - .and_then(|name| { + if let Some(tz) = Dropping::new(CFTimeZoneCopySystem()) { + if let Some(name) = Dropping::new(CFTimeZoneGetName(tz.0)) { let name = CFStringGetCStringPtr(name.0, kCFStringEncodingUTF8); - if name.is_null() { - None - } else { - Some(name) + if !name.is_null() { + let name = std::ffi::CStr::from_ptr(name); + if let Ok(name) = name.to_str() { + return Ok(name.to_owned()); + } } - }) - .and_then(|name| std::ffi::CStr::from_ptr(name).to_str().ok()) - .map(|name| name.to_owned()) - .ok_or(crate::GetTimezoneError::OsError) + } + } } + Err(crate::GetTimezoneError::OsError) } struct Dropping(*const T);