diff src/libpam/answer.rs @ 93:efc2b56c8928

Remove undefined behavior per MIRI. This replaces a bunch of raw pointers with NonNull and removes all the undefined behavior that we can find with MIRI. We also remove the `SecureString` dependency (since it doesn't work with MIRI, and because it's not really necessary).
author Paul Fisher <paul@pfish.zone>
date Mon, 23 Jun 2025 13:02:58 -0400
parents 05291b601f0a
children b87100c5eed4
line wrap: on
line diff
--- a/src/libpam/answer.rs	Sun Jun 22 19:29:32 2025 -0400
+++ b/src/libpam/answer.rs	Mon Jun 23 13:02:58 2025 -0400
@@ -7,6 +7,7 @@
 use crate::{ErrorCode, Result};
 use std::ffi::CStr;
 use std::ops::{Deref, DerefMut};
+use std::ptr::NonNull;
 use std::{iter, mem, ptr, slice};
 
 /// The corridor via which the answer to Messages navigate through PAM.
@@ -27,7 +28,7 @@
         // all allocated answer memory.
         for (input, output) in iter::zip(value, outputs.iter_mut()) {
             match input {
-                OwnedMessage::MaskedPrompt(p) => TextAnswer::fill(output, p.answer()?.unsecure())?,
+                OwnedMessage::MaskedPrompt(p) => TextAnswer::fill(output, p.answer()?.as_ref())?,
                 OwnedMessage::Prompt(p) => TextAnswer::fill(output, &(p.answer()?))?,
                 OwnedMessage::Error(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?,
                 OwnedMessage::Info(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?,
@@ -107,7 +108,7 @@
     fn fill(dest: &mut Answer, text: &str) -> Result<()> {
         let allocated = memory::malloc_str(text)?;
         dest.free_contents();
-        dest.data = allocated.cast();
+        dest.data = allocated.as_ptr().cast();
         Ok(())
     }
 
@@ -168,29 +169,28 @@
     pub fn fill(dest: &mut Answer, data_and_type: (&[u8], u8)) -> Result<()> {
         let allocated = CBinaryData::alloc(data_and_type)?;
         dest.free_contents();
-        dest.data = allocated.cast();
+        dest.data = allocated.as_ptr().cast();
         Ok(())
     }
 
     /// Gets the binary data in this answer.
-    pub fn data(&self) -> Option<&CBinaryData> {
+    pub fn data(&self) -> Option<NonNull<CBinaryData>> {
         // SAFETY: We either got this data from PAM or allocated it ourselves.
         // Either way, we trust that it is either valid data or null.
-        unsafe { self.0.data.cast::<CBinaryData>().as_ref() }
+        NonNull::new(self.0.data.cast::<CBinaryData>())
     }
 
     /// Zeroes out the answer data, frees it, and points our data to `null`.
     ///
-    /// When this `TextAnswer` is part of an [`Answers`],
+    /// When this `BinaryAnswer` is part of an [`Answers`],
     /// this is optional (since that will perform the `free`),
     /// but it will clear potentially sensitive data.
     pub fn zero_contents(&mut self) {
         // SAFETY: We know that our data pointer is either valid or null.
         // Once we're done, it's null and the answer is safe.
         unsafe {
-            let data_ref = self.0.data.cast::<CBinaryData>().as_mut();
-            if let Some(d) = data_ref {
-                d.zero_contents()
+            if let Some(ptr) = NonNull::new(self.0.data) {
+                CBinaryData::zero_contents(ptr.cast())
             }
             memory::free(self.0.data);
             self.0.data = ptr::null_mut()
@@ -277,9 +277,13 @@
 
         if let [bin, radio] = &mut answers[..] {
             let up = unsafe { BinaryAnswer::upcast(bin) };
-            assert_eq!(BinaryData::from((&[1, 2, 3][..], 99)), up.data().into());
+            assert_eq!(BinaryData::from((&[1, 2, 3][..], 99)), unsafe {
+                CBinaryData::as_binary_data(up.data().unwrap())
+            });
             up.zero_contents();
-            assert_eq!(BinaryData::default(), up.data().into());
+            assert_eq!(BinaryData::default(), unsafe {
+                CBinaryData::as_binary_data(up.data().unwrap())
+            });
 
             assert_text_answer("beep boop", radio);
         } else {
@@ -309,7 +313,9 @@
         let real_data = BinaryData::new([1, 2, 3, 4, 5, 6, 7, 8], 9);
         BinaryAnswer::fill(answer, (&real_data).into()).expect("alloc should succeed");
         let bin_answer = unsafe { BinaryAnswer::upcast(answer) };
-        assert_eq!(real_data, bin_answer.data().into());
+        assert_eq!(real_data, unsafe {
+            CBinaryData::as_binary_data(bin_answer.data().unwrap())
+        });
         answer.free_contents();
         answer.free_contents();
         unsafe { memory::free(answer_ptr) }