diff src/libpam/answer.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 33b9622ed6d2
children 8f964b701652
line wrap: on
line diff
--- a/src/libpam/answer.rs	Sat Jul 05 21:49:27 2025 -0400
+++ b/src/libpam/answer.rs	Sat Jul 05 22:12:46 2025 -0400
@@ -5,9 +5,10 @@
 use crate::libpam::memory::{CHeapBox, CHeapPayload, CHeapString, Immovable};
 use crate::{ErrorCode, Result};
 use libpam_sys_helpers::memory::BinaryPayload;
-use std::ffi::{c_int, c_void, CStr};
+use std::ffi::{c_int, c_void, CStr, OsStr};
 use std::mem::ManuallyDrop;
 use std::ops::{Deref, DerefMut};
+use std::os::unix::ffi::OsStrExt;
 use std::ptr::NonNull;
 use std::{iter, ptr, slice};
 
@@ -31,10 +32,14 @@
         // all allocated answer memory.
         for (input, output) in iter::zip(value, outputs.iter_mut()) {
             match input {
-                OwnedExchange::MaskedPrompt(p) => TextAnswer::fill(output, p.answer()?.as_ref())?,
-                OwnedExchange::Prompt(p) => TextAnswer::fill(output, &(p.answer()?))?,
-                OwnedExchange::Error(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?,
-                OwnedExchange::Info(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?,
+                OwnedExchange::MaskedPrompt(p) => TextAnswer::fill(output, &p.answer()?)?,
+                OwnedExchange::Prompt(p) => TextAnswer::fill(output, &p.answer()?)?,
+                OwnedExchange::Error(p) => {
+                    TextAnswer::fill(output, p.answer().map(|_| "".as_ref())?)?
+                }
+                OwnedExchange::Info(p) => {
+                    TextAnswer::fill(output, p.answer().map(|_| "".as_ref())?)?
+                }
                 // If we're here, that means that we *got* a Linux-PAM
                 // question from PAM, so we're OK to answer it.
                 OwnedExchange::RadioPrompt(p) => TextAnswer::fill(output, &(p.answer()?))?,
@@ -98,16 +103,15 @@
 
 /// Generic version of answer data.
 ///
-/// This has the same structure as [`BinaryAnswer`](crate::libpam::answer::BinaryAnswer)
-/// and [`TextAnswer`](crate::libpam::answer::TextAnswer).
+/// This has the same structure as [`BinaryAnswer`] and [`TextAnswer`].
 #[repr(C)]
 #[derive(Debug, Default)]
 pub struct Answer {
     /// Owned pointer to the data returned in an answer.
     /// For most answers, this will be a
-    /// [`CHeapString`](crate::libpam::memory::CHeapString),
-    /// but for [`BinaryQAndA`](crate::conv::BinaryQAndA)s
-    /// (a Linux-PAM extension), this will be a [`CHeapBox`] of
+    /// [`CHeapString`](CHeapString), but for
+    /// [`BinaryQAndA`](crate::conv::BinaryQAndA)s (a Linux-PAM extension),
+    /// this will be a [`CHeapBox`] of
     /// [`CBinaryData`](crate::libpam::memory::CBinaryData).
     pub data: Option<CHeapBox<c_void>>,
     /// Unused. Just here for the padding.
@@ -131,8 +135,8 @@
     }
 
     /// Converts the `Answer` to a `TextAnswer` with the given text.
-    fn fill(dest: &mut Answer, text: &str) -> Result<()> {
-        let allocated = CHeapString::new(text)?;
+    fn fill(dest: &mut Answer, text: &OsStr) -> Result<()> {
+        let allocated = CHeapString::new(text.as_bytes());
         let _ = dest
             .data
             .replace(unsafe { CHeapBox::cast(allocated.into_box()) });
@@ -237,7 +241,7 @@
 
     macro_rules! answered {
         ($typ:ty, $msg:path, $data:expr) => {{
-            let qa = <$typ>::new("");
+            let qa = <$typ>::new("".as_ref());
             qa.set_answer(Ok($data));
             $msg(qa)
         }};
@@ -250,16 +254,16 @@
         assert_eq!("", up.contents().unwrap());
     }
 
-    fn round_trip(msgs: Vec<OwnedExchange>) -> Answers {
-        let n = msgs.len();
-        let sent = Answers::build(msgs).unwrap();
+    fn round_trip(exchanges: Vec<OwnedExchange>) -> Answers {
+        let n = exchanges.len();
+        let sent = Answers::build(exchanges).unwrap();
         unsafe { Answers::from_c_heap(NonNull::new_unchecked(sent.into_ptr()), n) }
     }
 
     #[test]
     fn test_round_trip() {
         let mut answers = round_trip(vec![
-            answered!(QAndA, OwnedExchange::Prompt, "whats going on".to_owned()),
+            answered!(QAndA, OwnedExchange::Prompt, "whats going on".into()),
             answered!(MaskedQAndA, OwnedExchange::MaskedPrompt, "well then".into()),
             answered!(ErrorMsg, OwnedExchange::Error, ()),
             answered!(InfoMsg, OwnedExchange::Info, ()),
@@ -285,11 +289,7 @@
         };
         let mut answers = round_trip(vec![
             binary_msg,
-            answered!(
-                RadioQAndA,
-                OwnedExchange::RadioPrompt,
-                "beep boop".to_owned()
-            ),
+            answered!(RadioQAndA, OwnedExchange::RadioPrompt, "beep boop".into()),
         ]);
 
         if let [bin, radio] = &mut answers[..] {
@@ -307,13 +307,19 @@
     #[test]
     fn test_text_answer() {
         let mut answer: CHeapBox<Answer> = CHeapBox::default();
-        TextAnswer::fill(&mut answer, "hello").unwrap();
+        TextAnswer::fill(&mut answer, "hello".as_ref()).unwrap();
         let zeroth_text = unsafe { TextAnswer::upcast(&mut answer) };
         let data = zeroth_text.contents().expect("valid");
         assert_eq!("hello", data);
         zeroth_text.zero_contents();
         zeroth_text.zero_contents();
-        TextAnswer::fill(&mut answer, "hell\0").expect_err("should error; contains nul");
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_text_answer_nul() {
+        TextAnswer::fill(&mut CHeapBox::default(), "hell\0".as_ref())
+            .expect_err("should error; contains nul");
     }
 
     #[test]