From a5a684cba5e764f919ca2457517ba05ebb19a739 Mon Sep 17 00:00:00 2001
From: chrysn <chrysn@fsfe.org>
Date: Thu, 22 Aug 2024 00:38:57 +0200
Subject: [PATCH 1/2] vfs: Buffer names for nul termination

This fixes a severe bug where file names were passed to the C API that
expects nul-termination.

Closes: https://github.com/RIOT-OS/rust-riot-wrappers/issues/93
---
 src/error.rs |  1 +
 src/vfs.rs   | 51 +++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 10252db..f4ea592 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -107,6 +107,7 @@ macro_rules! E {
 
 // See module level comment
 E!(EAGAIN);
+E!(EINVAL);
 E!(ENOMEM);
 E!(ENOSPC);
 E!(EOVERFLOW);
diff --git a/src/vfs.rs b/src/vfs.rs
index b5b3854..8a60699 100644
--- a/src/vfs.rs
+++ b/src/vfs.rs
@@ -20,6 +20,38 @@ use riot_sys::libc;
 use crate::error::{NegativeErrorExt, NumericError};
 use crate::helpers::{PointerToCStr, SliceToCStr};
 
+#[derive(Copy, Clone, Debug)]
+struct NameTooLong;
+#[derive(Copy, Clone, Debug)]
+struct NameContainsIllegalNull;
+
+impl From<NameTooLong> for NumericError {
+    fn from(_: NameTooLong) -> NumericError {
+        crate::error::ENOMEM
+    }
+}
+
+impl From<NameContainsIllegalNull> for NumericError {
+    fn from(_: NameContainsIllegalNull) -> NumericError {
+        crate::error::EINVAL
+    }
+}
+struct NameNullTerminated(heapless::String<{ riot_sys::VFS_NAME_MAX as usize + 1 }>);
+
+impl NameNullTerminated {
+    fn new(name: &str) -> Result<Self, NameTooLong> {
+        let mut buf = heapless::String::new();
+        buf.push_str(name).map_err(|_| NameTooLong)?;
+        buf.push_str("\0").map_err(|_| NameTooLong)?;
+        Ok(NameNullTerminated(buf))
+    }
+
+    fn as_cstr(&self) -> Result<&core::ffi::CStr, NameContainsIllegalNull> {
+        core::ffi::CStr::from_bytes_with_nul(self.0.as_str().as_bytes())
+            .map_err(|_| NameContainsIllegalNull)
+    }
+}
+
 /// A file handle
 #[derive(Debug)]
 pub struct File {
@@ -57,14 +89,10 @@ pub enum SeekFrom {
 impl File {
     /// Open a file in read-only mode.
     pub fn open(path: &str) -> Result<Self, NumericError> {
-        let fileno = unsafe {
-            riot_sys::vfs_open(
-                path as *const str as *const libc::c_char,
-                riot_sys::O_RDONLY as _,
-                0,
-            )
-        }
-        .negative_to_error()?;
+        let path = NameNullTerminated::new(path)?;
+        let fileno =
+            unsafe { riot_sys::vfs_open(path.as_cstr()?.as_ptr(), riot_sys::O_RDONLY as _, 0) }
+                .negative_to_error()?;
         Ok(File {
             fileno,
             _not_send_sync: PhantomData,
@@ -120,11 +148,10 @@ pub struct Dir(riot_sys::vfs_DIR, core::marker::PhantomPinned);
 
 impl Dir {
     pub fn open(dir: &str) -> Result<Self, NumericError> {
+        let dir = NameNullTerminated::new(dir)?;
         let mut dirp = MaybeUninit::uninit();
-        (unsafe {
-            riot_sys::vfs_opendir(dirp.as_mut_ptr(), dir as *const str as *const libc::c_char)
-        })
-        .negative_to_error()?;
+        (unsafe { riot_sys::vfs_opendir(dirp.as_mut_ptr(), dir.as_cstr()?.as_ptr()) })
+            .negative_to_error()?;
         let dirp = unsafe { dirp.assume_init() };
         Ok(Dir(dirp, core::marker::PhantomPinned))
     }

From 27a09f4e429299e22c38fbbd9acaa494b862b363 Mon Sep 17 00:00:00 2001
From: chrysn <chrysn@fsfe.org>
Date: Thu, 22 Aug 2024 00:52:05 +0200
Subject: [PATCH 2/2] vfs: Account for different char signenesses

---
 src/vfs.rs | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/vfs.rs b/src/vfs.rs
index 8a60699..f694cce 100644
--- a/src/vfs.rs
+++ b/src/vfs.rs
@@ -90,9 +90,10 @@ impl File {
     /// Open a file in read-only mode.
     pub fn open(path: &str) -> Result<Self, NumericError> {
         let path = NameNullTerminated::new(path)?;
-        let fileno =
-            unsafe { riot_sys::vfs_open(path.as_cstr()?.as_ptr(), riot_sys::O_RDONLY as _, 0) }
-                .negative_to_error()?;
+        let fileno = unsafe {
+            riot_sys::vfs_open(path.as_cstr()?.as_ptr() as _, riot_sys::O_RDONLY as _, 0)
+        }
+        .negative_to_error()?;
         Ok(File {
             fileno,
             _not_send_sync: PhantomData,
@@ -150,7 +151,7 @@ impl Dir {
     pub fn open(dir: &str) -> Result<Self, NumericError> {
         let dir = NameNullTerminated::new(dir)?;
         let mut dirp = MaybeUninit::uninit();
-        (unsafe { riot_sys::vfs_opendir(dirp.as_mut_ptr(), dir.as_cstr()?.as_ptr()) })
+        (unsafe { riot_sys::vfs_opendir(dirp.as_mut_ptr(), dir.as_cstr()?.as_ptr() as _) })
             .negative_to_error()?;
         let dirp = unsafe { dirp.assume_init() };
         Ok(Dir(dirp, core::marker::PhantomPinned))