Return Path instead of String for safety and portability

Co-authored-by: Zachary Dremann <dremann@gmail.com>
rbspy · Nov 2, 2021 · 4b89807 · 4b89807
1 parent e2b3ffa
commit 4b89807
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 38 deletions.
diff --git a/src/freebsd_maps/mod.rs b/src/freebsd_maps/mod.rs
@@ -6,6 +6,7 @@ mod ptrace;
 use libc::{c_int, pid_t};
 use std::convert::From;
 use std::iter::Iterator;
+use std::path::{Path, PathBuf};
 
 pub type Pid = pid_t;
 
@@ -16,7 +17,7 @@ pub struct MapRange {
     protection: c_int,
     offset: usize,
     vnode: usize,
-    pathname: Option<String>,
+    pathname: Option<PathBuf>,
 }
 
 impl MapRange {
@@ -26,8 +27,8 @@ impl MapRange {
     pub fn start(&self) -> usize {
         self.range_start
     }
-    pub fn filename(&self) -> &Option<String> {
-        &self.pathname
+    pub fn filename(&self) -> Option<&Path> {
+        self.pathname.as_deref()
     }
 
     pub fn is_read(&self) -> bool {
@@ -73,7 +74,7 @@ fn test_map_from_invoked_binary_present() -> () {
 
     let maybe_cat_region = maps
         .iter()
-        .find(|x| x.filename() == &Some(String::from("/bin/cat")));
+        .find(|map| map.filename() == Some(&PathBuf::from("/bin/cat")));
 
     assert!(
         maybe_cat_region.is_some(),

diff --git a/src/freebsd_maps/ptrace.rs b/src/freebsd_maps/ptrace.rs
@@ -1,9 +1,12 @@
 use libc::{c_char, c_int};
 use libc::{waitpid, PT_ATTACH, PT_DETACH, PT_VM_ENTRY, WIFSTOPPED};
 use std::convert::From;
-use std::ffi::CStr;
+use std::ffi::{CStr, OsStr};
+use std::io;
 use std::iter::Iterator;
-use std::{io, ptr};
+use std::os::unix::ffi::OsStrExt;
+use std::path::PathBuf;
+use std::ptr;
 
 use super::bindings::{caddr_t, ptrace_vm_entry};
 use super::Pid;
@@ -38,7 +41,7 @@ pub struct VmEntry {
     pub pve_pathlen: u32,
     pub pve_fileid: i64,
     pub pve_fsid: u32,
-    pub pve_path: Option<String>,
+    pub pve_path: Option<PathBuf>,
 }
 
 impl From<ptrace_vm_entry> for VmEntry {
@@ -126,15 +129,16 @@ impl Iterator for VmEntryIterator {
     }
 }
 
-fn string_from_cstr_ptr(pointer: *const c_char) -> Option<String> {
+fn string_from_cstr_ptr(pointer: *const c_char) -> Option<PathBuf> {
     if pointer.is_null() {
         None
     } else {
         unsafe {
-            let result = CStr::from_ptr(pointer).to_string_lossy().into_owned();
+            let cstr = CStr::from_ptr(pointer);
+            let osstr = OsStr::from_bytes(cstr.to_bytes());
 
-            if result.len() > 0 {
-                Some(result)
+            if osstr.len() > 0 {
+                Some(PathBuf::from(osstr))
             } else {
                 None
             }

diff --git a/src/linux_maps.rs b/src/linux_maps.rs
@@ -2,6 +2,7 @@ use libc;
 use std;
 use std::fs::File;
 use std::io::Read;
+use std::path::{Path, PathBuf};
 
 pub type Pid = libc::pid_t;
 
@@ -17,7 +18,7 @@ pub struct MapRange {
     pub dev: String,
     pub flags: String,
     pub inode: usize,
-    pathname: Option<String>,
+    pathname: Option<PathBuf>,
 }
 
 impl MapRange {
@@ -30,8 +31,8 @@ impl MapRange {
         self.range_start
     }
     /// Returns the filename of the loaded module
-    pub fn filename(&self) -> &Option<String> {
-        &self.pathname
+    pub fn filename(&self) -> Option<&Path> {
+        self.pathname.as_deref()
     }
     /// Returns whether this range contains executable code
     pub fn is_exec(&self) -> bool {
@@ -74,6 +75,11 @@ fn parse_proc_maps(contents: &str) -> Vec<MapRange> {
         let offset = split.next().unwrap();
         let dev = split.next().unwrap();
         let inode = split.next().unwrap();
+        let pathname = match Some(split.collect::<Vec<&str>>().join(" ")).filter(|x| !x.is_empty())
+        {
+            Some(s) => Some(PathBuf::from(s)),
+            None => None,
+        };
 
         vec.push(MapRange {
             range_start: usize::from_str_radix(range_start, 16).unwrap(),
@@ -82,7 +88,7 @@ fn parse_proc_maps(contents: &str) -> Vec<MapRange> {
             dev: dev.to_string(),
             flags: flags.to_string(),
             inode: usize::from_str_radix(inode, 10).unwrap(),
-            pathname: Some(split.collect::<Vec<&str>>().join(" ")).filter(|x| !x.is_empty()),
+            pathname,
         });
     }
     vec
@@ -100,7 +106,7 @@ fn test_parse_maps() {
             dev: "00:14".to_string(),
             flags: "r-xp".to_string(),
             inode: 205736,
-            pathname: Some("/usr/bin/fish".to_string()),
+            pathname: Some(PathBuf::from("/usr/bin/fish")),
         },
         MapRange {
             range_start: 0x00708000,
@@ -118,7 +124,7 @@ fn test_parse_maps() {
             dev: "00:00".to_string(),
             flags: "rw-p".to_string(),
             inode: 0,
-            pathname: Some("[heap]".to_string()),
+            pathname: Some(PathBuf::from("[heap]")),
         },
         MapRange {
             range_start: 0x7f438053b000,
@@ -127,9 +133,9 @@ fn test_parse_maps() {
             dev: "fd:01".to_string(),
             flags: "r--p".to_string(),
             inode: 59034409,
-            pathname: Some(
-                "/usr/lib/x86_64-linux-gnu/libgmodule-2.0.so.0.4200.6 (deleted)".to_string(),
-            ),
+            pathname: Some(PathBuf::from(
+                "/usr/lib/x86_64-linux-gnu/libgmodule-2.0.so.0.4200.6 (deleted)",
+            )),
         },
     ];
     assert_eq!(vec, expected);

diff --git a/src/mac_maps/mod.rs b/src/mac_maps/mod.rs
@@ -14,6 +14,7 @@ use mach::vm_types::{mach_vm_address_t, mach_vm_size_t};
 use std;
 use std::io;
 use std::mem;
+use std::path::{Path, PathBuf};
 
 mod dyld_bindings;
 use self::dyld_bindings::{
@@ -28,7 +29,7 @@ pub struct MapRange {
     info: vm_region_basic_info_data_t,
     start: mach_vm_address_t,
     count: mach_msg_type_number_t,
-    filename: Option<String>,
+    filename: Option<PathBuf>,
 }
 
 #[derive(Debug, Clone)]
@@ -75,8 +76,8 @@ impl MapRange {
     pub fn start(&self) -> usize {
         self.start as usize
     }
-    pub fn filename(&self) -> &Option<String> {
-        &self.filename
+    pub fn filename(&self) -> Option<&Path> {
+        self.filename.as_deref()
     }
 
     pub fn is_read(&self) -> bool {
@@ -141,7 +142,7 @@ fn mach_vm_region(
         return None;
     }
     let filename = match regionfilename(pid, address) {
-        Ok(x) => Some(x),
+        Ok(s) => Some(PathBuf::from(s.as_str())),
         _ => None,
     };
     Some(MapRange {
@@ -172,7 +173,7 @@ pub fn task_for_pid(pid: Pid) -> io::Result<mach_port_name_t> {
 
 #[derive(Debug, Clone)]
 pub struct DyldInfo {
-    pub filename: String,
+    pub filename: PathBuf,
     pub address: usize,
     pub file_mod_date: usize,
     pub segment: segment_command_64,
@@ -333,7 +334,7 @@ pub fn get_dyld_info(pid: Pid) -> io::Result<Vec<DyldInfo>> {
                 if command.cmd == 0x19 {
                     command.vmaddr += slide;
                     vec.push(DyldInfo {
-                        filename: filename.clone(),
+                        filename: PathBuf::from(filename.clone()),
                         address: module.imageLoadAddress as usize,
                         file_mod_date: module.imageFileModDate,
                         segment: command,

diff --git a/src/win_maps.rs b/src/win_maps.rs
@@ -1,8 +1,8 @@
-use libc::wcslen;
 use std;
 use std::ffi::{OsStr, OsString};
 use std::io;
 use std::os::windows::ffi::{OsStrExt, OsStringExt};
+use std::path::{Path, PathBuf};
 use std::ptr::null_mut;
 
 use winapi::shared::minwindef::{DWORD, FALSE};
@@ -21,7 +21,7 @@ pub type Pid = u32;
 pub struct MapRange {
     base_addr: usize,
     base_size: usize,
-    pathname: Option<String>,
+    pathname: Option<PathBuf>,
 }
 
 impl MapRange {
@@ -31,8 +31,8 @@ impl MapRange {
     pub fn start(&self) -> usize {
         self.base_addr
     }
-    pub fn filename(&self) -> &Option<String> {
-        &self.pathname
+    pub fn filename(&self) -> Option<&Path> {
+        self.pathname.as_deref()
     }
     pub fn is_exec(&self) -> bool {
         true
@@ -68,7 +68,7 @@ pub fn get_process_maps(pid: Pid) -> io::Result<Vec<MapRange>> {
             vec.push(MapRange {
                 base_addr: module.modBaseAddr as usize,
                 base_size: module.modBaseSize as usize,
-                pathname: Some(wstr_to_string(module.szExePath.as_ptr())),
+                pathname: Some(PathBuf::from(wstr_to_string(&module.szExePath))),
             });
 
             success = Module32NextW(handle, &mut module);
@@ -88,7 +88,7 @@ pub struct SymbolLoader {
 
 pub struct SymbolModule<'a> {
     pub parent: &'a SymbolLoader,
-    pub filename: &'a str,
+    pub filename: &'a Path,
     pub base: u64,
 }
 
@@ -121,12 +121,12 @@ impl SymbolLoader {
     }
 
     /// Loads symbols for filename, returns a SymbolModule structure that must be kept alive
-    pub fn load_module<'a>(&'a self, filename: &'a str) -> io::Result<SymbolModule<'a>> {
+    pub fn load_module<'a>(&'a self, filename: &'a Path) -> io::Result<SymbolModule<'a>> {
         unsafe {
             let base = SymLoadModuleExW(
                 self.process,
                 null_mut(),
-                string_to_wstr(filename).as_ptr(),
+                path_to_wstr(filename).as_ptr(),
                 null_mut(),
                 0,
                 0,
@@ -162,12 +162,19 @@ impl<'a> Drop for SymbolModule<'a> {
     }
 }
 
-fn wstr_to_string(ptr: *const u16) -> String {
-    let slice = unsafe { std::slice::from_raw_parts(ptr, wcslen(ptr)) };
-    OsString::from_wide(slice).to_string_lossy().into_owned()
+fn wstr_to_string(full: &[u16]) -> OsString {
+    let len = full.iter().position(|&x| x == 0).unwrap_or(full.len());
+    OsString::from_wide(&full[..len])
 }
 
-pub fn string_to_wstr(val: &str) -> Vec<u16> {
+fn string_to_wstr(val: &str) -> Vec<u16> {
+    OsStr::new(val)
+        .encode_wide()
+        .chain(std::iter::once(0))
+        .collect()
+}
+
+fn path_to_wstr(val: &Path) -> Vec<u16> {
     OsStr::new(val)
         .encode_wide()
         .chain(std::iter::once(0))