Skip to content

Commit

Permalink
Fix potential use after free in MacOS / iOS impl
Browse files Browse the repository at this point in the history
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.

<#50 (comment)>
  • Loading branch information
Kijewski committed Aug 16, 2022
1 parent fe425a8 commit 0819f92
Showing 1 changed file with 10 additions and 11 deletions.
21 changes: 10 additions & 11 deletions src/tz_macos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,19 @@ use core_foundation_sys::timezone::{CFTimeZoneCopySystem, CFTimeZoneGetName};

pub(crate) fn get_timezone_inner() -> Result<String, crate::GetTimezoneError> {
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<T>(*const T);
Expand Down

0 comments on commit 0819f92

Please sign in to comment.