From 89ce25f07785b1483cae7c4f34ee637d660726b0 Mon Sep 17 00:00:00 2001 From: s1341 Date: Thu, 4 Jan 2024 09:07:53 +0200 Subject: [PATCH 1/2] RangeDetails: don't return dangling pointers (#121) * rangedetails: don't return dangling pointers * size is a usize --- frida-gum/src/memory_range.rs | 2 ++ frida-gum/src/range_details.rs | 56 +++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/frida-gum/src/memory_range.rs b/frida-gum/src/memory_range.rs index 4408613..e9e1dc8 100644 --- a/frida-gum/src/memory_range.rs +++ b/frida-gum/src/memory_range.rs @@ -43,6 +43,8 @@ pub struct ScanResult { pub address: usize, pub size: usize, } + +#[derive(Clone)] pub struct MemoryRange { pub(crate) memory_range: gum_sys::GumMemoryRange, } diff --git a/frida-gum/src/range_details.rs b/frida-gum/src/range_details.rs index e2b23b3..5fdea78 100644 --- a/frida-gum/src/range_details.rs +++ b/frida-gum/src/range_details.rs @@ -23,7 +23,7 @@ use { use alloc::boxed::Box; /// The memory protection of an unassociated page. -#[derive(FromPrimitive)] +#[derive(Clone, FromPrimitive)] #[repr(u32)] pub enum PageProtection { NoAccess = gum_sys::_GumPageProtection_GUM_PAGE_NO_ACCESS as u32, @@ -40,32 +40,43 @@ pub enum PageProtection { } /// The file association to a page. +#[derive(Clone)] pub struct FileMapping<'a> { - file_mapping: *const gum_sys::GumFileMapping, + path: &'a str, + size: usize, + offset: u64, phantom: PhantomData<&'a gum_sys::GumFileMapping>, } impl<'a> FileMapping<'a> { - pub(crate) fn from_raw(file: *const gum_sys::GumFileMapping) -> Self { - Self { - file_mapping: file, - phantom: PhantomData, + pub(crate) fn from_raw(file: *const gum_sys::GumFileMapping) -> Option { + if file.is_null() { + None + } else { + Some(unsafe { + Self { + path: CStr::from_ptr((*file).path).to_str().unwrap(), + size: (*file).size as usize, + offset: (*file).offset, + phantom: PhantomData, + } + }) } } /// The path of the file mapping on disk. pub fn path(&self) -> &str { - unsafe { CStr::from_ptr((*self.file_mapping).path).to_str().unwrap() } + self.path } /// The offset into the file mapping. pub fn offset(&self) -> u64 { - unsafe { (*self.file_mapping).offset } + self.offset } /// The size of the mapping. - pub fn size(&self) -> u64 { - unsafe { (*self.file_mapping).size as u64 } + pub fn size(&self) -> usize { + self.size as usize } } @@ -104,15 +115,21 @@ unsafe extern "C" fn enumerate_ranges_stub( /// Details a range of virtual memory. pub struct RangeDetails<'a> { - range_details: *const gum_sys::GumRangeDetails, + range: MemoryRange, + protection: PageProtection, + file: Option>, phantom: PhantomData<&'a gum_sys::GumRangeDetails>, } impl<'a> RangeDetails<'a> { pub(crate) fn from_raw(range_details: *const gum_sys::GumRangeDetails) -> Self { - Self { - range_details, - phantom: PhantomData, + unsafe { + Self { + range: MemoryRange::from_raw((*range_details).range), + protection: num::FromPrimitive::from_u32((*range_details).protection).unwrap(), + file: FileMapping::from_raw((*range_details).file), + phantom: PhantomData, + } } } @@ -154,21 +171,16 @@ impl<'a> RangeDetails<'a> { /// The range of memory that is detailed. pub fn memory_range(&self) -> MemoryRange { - unsafe { MemoryRange::from_raw((*self.range_details).range) } + self.range.clone() } /// The page protection of the range. pub fn protection(&self) -> PageProtection { - let protection = unsafe { (*self.range_details).protection }; - num::FromPrimitive::from_u32(protection).unwrap() + self.protection.clone() } /// The associated file mapping, if present. pub fn file_mapping(&self) -> Option { - if self.range_details.is_null() { - None - } else { - Some(unsafe { FileMapping::from_raw((*self.range_details).file) }) - } + self.file.clone() } } From 92cde4ec112ff404fd2a4377dc926074b9319ed6 Mon Sep 17 00:00:00 2001 From: s1341 Date: Thu, 4 Jan 2024 09:20:46 +0200 Subject: [PATCH 2/2] Fix rangedetails for real (#122) * rangedetails: don't return dangling pointers * size is a usize * Actually build the RangeDetails at the right time * Use lifetime parameter --- frida-gum/src/range_details.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/frida-gum/src/range_details.rs b/frida-gum/src/range_details.rs index 5fdea78..90d87fa 100644 --- a/frida-gum/src/range_details.rs +++ b/frida-gum/src/range_details.rs @@ -80,9 +80,9 @@ impl<'a> FileMapping<'a> { } } -struct SaveRangeDetailsByAddressContext { +struct SaveRangeDetailsByAddressContext<'a> { address: u64, - details: *const gum_sys::GumRangeDetails, + details: Option>, } unsafe extern "C" fn save_range_details_by_address( @@ -94,7 +94,7 @@ unsafe extern "C" fn save_range_details_by_address( let start = (*range).base_address as u64; let end = start + (*range).size as u64; if start <= context.address && context.address < end { - context.details = details; + context.details = Some(RangeDetails::from_raw(details)); return 0; } @@ -137,7 +137,7 @@ impl<'a> RangeDetails<'a> { pub fn with_address(address: u64) -> Option> { let mut context = SaveRangeDetailsByAddressContext { address, - details: core::ptr::null_mut(), + details: None, }; unsafe { gum_sys::gum_process_enumerate_ranges( @@ -147,11 +147,7 @@ impl<'a> RangeDetails<'a> { ); } - if !context.details.is_null() { - Some(RangeDetails::from_raw(context.details)) - } else { - None - } + context.details } /// Enumerate all ranges which match the given [`PageProtection`], calling the callback