diff src/libpam/memory.rs @ 143:ebb71a412b58

Turn everything into OsString and Just Walk Out! for strings with nul. To reduce the hazard surface of the API, this replaces most uses of &str with &OsStr (and likewise with String/OsString). Also, I've decided that instead of dealing with callers putting `\0` in their parameters, I'm going to follow the example of std::env and Just Walk Out! (i.e., panic!()). This makes things a lot less annoying for both me and (hopefully) users.
author Paul Fisher <paul@pfish.zone>
date Sat, 05 Jul 2025 22:12:46 -0400
parents a508a69c068a
children 4b3a5095f68c
line wrap: on
line diff
--- a/src/libpam/memory.rs	Sat Jul 05 21:49:27 2025 -0400
+++ b/src/libpam/memory.rs	Sat Jul 05 22:12:46 2025 -0400
@@ -1,12 +1,11 @@
 //! Things for dealing with memory.
 
-use crate::ErrorCode;
-use crate::Result;
 use libpam_sys_helpers::memory::{Buffer, OwnedBinaryPayload};
-use std::ffi::{c_char, CStr, CString};
+use std::ffi::{c_char, CStr, CString, OsStr, OsString};
 use std::marker::{PhantomData, PhantomPinned};
 use std::mem::ManuallyDrop;
 use std::ops::{Deref, DerefMut};
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::ptr::NonNull;
 use std::{mem, ptr, slice};
 
@@ -33,15 +32,16 @@
 pub struct Immovable(pub PhantomData<(*mut u8, PhantomPinned)>);
 
 /// Safely converts a `&str` option to a `CString` option.
-pub fn option_cstr(prompt: Option<&str>) -> Result<Option<CString>> {
-    prompt
-        .map(CString::new)
-        .transpose()
-        .map_err(|_| ErrorCode::ConversationError)
+pub fn option_cstr(prompt: Option<&[u8]>) -> Option<CString> {
+    prompt.map(|p| CString::new(p).expect("nul is not allowed"))
+}
+
+pub fn option_cstr_os(prompt: Option<&OsStr>) -> Option<CString> {
+    option_cstr(prompt.map(OsStr::as_bytes))
 }
 
 /// Gets the pointer to the given CString, or a null pointer if absent.
-pub fn prompt_ptr(prompt: Option<&CString>) -> *const c_char {
+pub fn prompt_ptr(prompt: Option<&CStr>) -> *const c_char {
     match prompt {
         Some(c_str) => c_str.as_ptr(),
         None => ptr::null(),
@@ -168,10 +168,10 @@
 
 impl CHeapString {
     /// Creates a new C heap string with the given contents.
-    pub fn new(text: &str) -> Result<Self> {
-        let data = text.as_bytes();
+    pub fn new(text: impl AsRef<[u8]>) -> Self {
+        let data = text.as_ref();
         if data.contains(&0) {
-            return Err(ErrorCode::ConversationError);
+            panic!("you're not allowed to create a cstring with a nul inside!");
         }
         // +1 for the null terminator
         let data_alloc: NonNull<c_char> = calloc(data.len() + 1);
@@ -179,7 +179,7 @@
         unsafe {
             let dest = slice::from_raw_parts_mut(data_alloc.as_ptr().cast(), data.len());
             dest.copy_from_slice(data);
-            Ok(Self(CHeapBox::from_ptr(data_alloc)))
+            Self(CHeapBox::from_ptr(data_alloc))
         }
     }
 
@@ -247,16 +247,13 @@
 /// # Safety
 ///
 /// It's on you to provide a valid string.
-pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result<Option<String>> {
-    let borrowed = match NonNull::new(result_ptr.cast_mut()) {
-        Some(data) => Some(
-            CStr::from_ptr(data.as_ptr())
-                .to_str()
-                .map_err(|_| ErrorCode::ConversationError)?,
-        ),
-        None => return Ok(None),
-    };
-    Ok(borrowed.map(String::from))
+pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Option<OsString> {
+    NonNull::new(result_ptr.cast_mut())
+        .map(NonNull::as_ptr)
+        .map(|p| CStr::from_ptr(p))
+        .map(CStr::to_bytes)
+        .map(Vec::from)
+        .map(OsString::from_vec)
 }
 
 #[cfg(test)]
@@ -285,37 +282,46 @@
         drop(dropbox);
         assert_eq!(3, drop_count.get());
     }
+
     #[test]
     fn test_strings() {
-        let str = CHeapString::new("hello there").unwrap();
+        let str = CHeapString::new("hello there");
         let str_ptr = str.into_ptr().as_ptr();
-        CHeapString::new("hell\0 there").unwrap_err();
         unsafe {
             let copied = copy_pam_string(str_ptr).unwrap();
-            assert_eq!("hello there", copied.unwrap());
+            assert_eq!("hello there", copied);
             CHeapString::zero(NonNull::new(str_ptr).unwrap());
             let idx_three = str_ptr.add(3).as_mut().unwrap();
             *idx_three = 0x80u8 as i8;
-            let zeroed = copy_pam_string(str_ptr).unwrap().unwrap();
+            let zeroed = copy_pam_string(str_ptr).unwrap();
             assert!(zeroed.is_empty());
             let _ = CHeapString::from_ptr(str_ptr);
         }
     }
 
     #[test]
+    #[should_panic]
+    fn test_nul_string() {
+        CHeapString::new("hell\0 there");
+    }
+
+    #[test]
     fn test_option_str() {
-        let good = option_cstr(Some("whatever")).unwrap();
+        let good = option_cstr(Some("whatever".as_ref()));
         assert_eq!("whatever", good.unwrap().to_str().unwrap());
-        let no_str = option_cstr(None).unwrap();
+        let no_str = option_cstr(None);
         assert!(no_str.is_none());
-        let bad_str = option_cstr(Some("what\0ever")).unwrap_err();
-        assert_eq!(ErrorCode::ConversationError, bad_str);
+    }
+    #[test]
+    #[should_panic]
+    fn test_nul_cstr() {
+        option_cstr(Some("what\0ever".as_ref()));
     }
 
     #[test]
     fn test_prompt() {
         let prompt_cstr = CString::new("good").ok();
-        let prompt = prompt_ptr(prompt_cstr.as_ref());
+        let prompt = prompt_ptr(prompt_cstr.as_deref());
         assert!(!prompt.is_null());
         let no_prompt = prompt_ptr(None);
         assert!(no_prompt.is_null());