Handle non-NUL-terminated strings in SocketAddrUnix. (#1371)

sunfishcode · Kijewski · web-flow · commit 4ad970591b02 · 2025-03-03T11:21:36.000-08:00
* Handle non-NUL-terminated strings in `SocketAddrUnix`. Unix-domain socket address can be long enough that the NUL terminator does not fit. Handle this case by making `path()` return a `Cow<CStr>` and adding a NUL terminator as needed. Also add a `path_bytes()` function for returning the raw bytes. Fixes #1316. * Allocated `SocketAddrUnix::path()` with correct length (#1372) Using `.to_owned()` + `.push()` will cause a reallocation, because the initially allocated array with be one byte too short. We can use `CString::from_vec_with_nul_unchecked()` because it is a known invariant that the input does not contain any `NUL`s, not even the terminating `NUL`. --------- Co-authored-by: René Kijewski <Kijewski@users.noreply.github.com>
diff --git a/src/backend/libc/net/addr.rs b/src/backend/libc/net/addr.rs
@@ -13,6 +13,8 @@ use {
     core::hash::{Hash, Hasher},
     core::slice,
 };
+#[cfg(all(unix, feature = "alloc"))]
+use {crate::ffi::CString, alloc::borrow::Cow, alloc::vec::Vec};
 
 /// `struct sockaddr_un`
 #[cfg(unix)]
@@ -35,9 +37,12 @@ impl SocketAddrUnix {
     #[inline]
     fn _new(path: &CStr) -> io::Result<Self> {
         let mut unix = Self::init();
-        let bytes = path.to_bytes_with_nul();
+        let mut bytes = path.to_bytes_with_nul();
         if bytes.len() > unix.sun_path.len() {
-            return Err(io::Errno::NAMETOOLONG);
+            bytes = path.to_bytes(); // without NUL
+            if bytes.len() > unix.sun_path.len() {
+                return Err(io::Errno::NAMETOOLONG);
+            }
         }
         for (i, b) in bytes.iter().enumerate() {
             unix.sun_path[i] = *b as c::c_char;
@@ -129,12 +134,52 @@ impl SocketAddrUnix {
 
     /// For a filesystem path address, return the path.
     #[inline]
-    pub fn path(&self) -> Option<&CStr> {
+    #[cfg(feature = "alloc")]
+    #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
+    pub fn path(&self) -> Option<Cow<'_, CStr>> {
         let bytes = self.bytes()?;
         if !bytes.is_empty() && bytes[0] != 0 {
-            // SAFETY: `from_bytes_with_nul_unchecked` since the string is
-            // NUL-terminated.
-            Some(unsafe { CStr::from_bytes_with_nul_unchecked(bytes) })
+            if self.unix.sun_path.len() == bytes.len() {
+                // SAFETY: no NULs are contained in bytes
+                unsafe { self.path_with_termination(bytes) }
+            } else {
+                // SAFETY: `from_bytes_with_nul_unchecked` since the string is
+                // NUL-terminated.
+                Some(unsafe { CStr::from_bytes_with_nul_unchecked(bytes) }.into())
+            }
+        } else {
+            None
+        }
+    }
+
+    /// If the `sun_path` field is not NUL-terminated, terminate it.
+    ///
+    /// SAFETY: the input `bytes` must not contain any NULs
+    #[cfg(feature = "alloc")]
+    unsafe fn path_with_termination(&self, bytes: &[u8]) -> Option<Cow<'_, CStr>> {
+        let mut owned = Vec::with_capacity(bytes.len() + 1);
+        owned.extend_from_slice(bytes);
+        owned.push(b'\0');
+        // SAFETY: `from_vec_with_nul_unchecked` since the string is
+        // NUL-terminated and `bytes` does not conain any NULs.
+        Some(Cow::Owned(
+            CString::from_vec_with_nul_unchecked(owned).into(),
+        ))
+    }
+
+    /// For a filesystem path address, return the path as a byte sequence,
+    /// excluding the NUL terminator.
+    #[inline]
+    pub fn path_bytes(&self) -> Option<&[u8]> {
+        let bytes = self.bytes()?;
+        if !bytes.is_empty() && bytes[0] != 0 {
+            if self.unix.sun_path.len() == self.len() - offsetof_sun_path() {
+                // There is no NUL terminator.
+                Some(bytes)
+            } else {
+                // Remove the NUL terminator.
+                Some(&bytes[..bytes.len() - 1])
+            }
         } else {
             None
         }
@@ -231,16 +276,21 @@ impl Hash for SocketAddrUnix {
 #[cfg(unix)]
 impl fmt::Debug for SocketAddrUnix {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        #[cfg(feature = "alloc")]
         if let Some(path) = self.path() {
-            path.fmt(f)
-        } else {
-            #[cfg(linux_kernel)]
-            if let Some(name) = self.abstract_name() {
-                return name.fmt(f);
+            return path.fmt(f);
+        }
+        if let Some(bytes) = self.path_bytes() {
+            if let Ok(s) = core::str::from_utf8(bytes) {
+                return s.fmt(f);
             }
-
-            "(unnamed)".fmt(f)
+            return bytes.fmt(f);
+        }
+        #[cfg(linux_kernel)]
+        if let Some(name) = self.abstract_name() {
+            return name.fmt(f);
         }
+        "(unnamed)".fmt(f)
     }
 }
 
diff --git a/src/backend/linux_raw/net/addr.rs b/src/backend/linux_raw/net/addr.rs
@@ -14,6 +14,8 @@ use crate::{io, path};
 use core::cmp::Ordering;
 use core::hash::{Hash, Hasher};
 use core::{fmt, slice};
+#[cfg(feature = "alloc")]
+use {crate::ffi::CString, alloc::borrow::Cow, alloc::vec::Vec};
 
 /// `struct sockaddr_un`
 #[derive(Clone)]
@@ -33,9 +35,12 @@ impl SocketAddrUnix {
     #[inline]
     fn _new(path: &CStr) -> io::Result<Self> {
         let mut unix = Self::init();
-        let bytes = path.to_bytes_with_nul();
+        let mut bytes = path.to_bytes_with_nul();
         if bytes.len() > unix.sun_path.len() {
-            return Err(io::Errno::NAMETOOLONG);
+            bytes = path.to_bytes(); // without NUL
+            if bytes.len() > unix.sun_path.len() {
+                return Err(io::Errno::NAMETOOLONG);
+            }
         }
         for (i, b) in bytes.iter().enumerate() {
             unix.sun_path[i] = bitcast!(*b);
@@ -91,12 +96,52 @@ impl SocketAddrUnix {
 
     /// For a filesystem path address, return the path.
     #[inline]
-    pub fn path(&self) -> Option<&CStr> {
+    #[cfg(feature = "alloc")]
+    #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
+    pub fn path(&self) -> Option<Cow<'_, CStr>> {
         let bytes = self.bytes()?;
         if !bytes.is_empty() && bytes[0] != 0 {
-            // SAFETY: `from_bytes_with_nul_unchecked` since the string is
-            // NUL-terminated.
-            Some(unsafe { CStr::from_bytes_with_nul_unchecked(bytes) })
+            if self.unix.sun_path.len() == bytes.len() {
+                // SAFETY: no NULs are contained in bytes
+                unsafe { self.path_with_termination(bytes) }
+            } else {
+                // SAFETY: `from_bytes_with_nul_unchecked` since the string is
+                // NUL-terminated.
+                Some(unsafe { CStr::from_bytes_with_nul_unchecked(bytes) }.into())
+            }
+        } else {
+            None
+        }
+    }
+
+    /// If the `sun_path` field is not NUL-terminated, terminate it.
+    ///
+    /// SAFETY: the input `bytes` must not contain any NULs
+    #[cfg(feature = "alloc")]
+    unsafe fn path_with_termination(&self, bytes: &[u8]) -> Option<Cow<'_, CStr>> {
+        let mut owned = Vec::with_capacity(bytes.len() + 1);
+        owned.extend_from_slice(bytes);
+        owned.push(b'\0');
+        // SAFETY: `from_vec_with_nul_unchecked` since the string is
+        // NUL-terminated and `bytes` does not conain any NULs.
+        Some(Cow::Owned(
+            CString::from_vec_with_nul_unchecked(owned).into(),
+        ))
+    }
+
+    /// For a filesystem path address, return the path as a byte sequence,
+    /// excluding the NUL terminator.
+    #[inline]
+    pub fn path_bytes(&self) -> Option<&[u8]> {
+        let bytes = self.bytes()?;
+        if !bytes.is_empty() && bytes[0] != 0 {
+            if self.unix.sun_path.len() == self.len() - offsetof_sun_path() {
+                // There is no NUL terminator.
+                Some(bytes)
+            } else {
+                // Remove the NUL terminator.
+                Some(&bytes[..bytes.len() - 1])
+            }
         } else {
             None
         }
@@ -178,13 +223,20 @@ impl Hash for SocketAddrUnix {
 
 impl fmt::Debug for SocketAddrUnix {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        #[cfg(feature = "alloc")]
         if let Some(path) = self.path() {
-            path.fmt(f)
-        } else if let Some(name) = self.abstract_name() {
-            name.fmt(f)
-        } else {
-            "(unnamed)".fmt(f)
+            return path.fmt(f);
+        }
+        if let Some(bytes) = self.path_bytes() {
+            if let Ok(s) = core::str::from_utf8(bytes) {
+                return s.fmt(f);
+            }
+            return bytes.fmt(f);
+        }
+        if let Some(name) = self.abstract_name() {
+            return name.fmt(f);
         }
+        "(unnamed)".fmt(f)
     }
 }
 
diff --git a/tests/net/addr.rs b/tests/net/addr.rs
@@ -31,19 +31,19 @@ fn test_unix_addr() {
 
     assert_eq!(
         SocketAddrUnix::new("/").unwrap().path().unwrap(),
-        cstr!("/")
+        cstr!("/").into()
     );
     assert_eq!(
         SocketAddrUnix::new("//").unwrap().path().unwrap(),
-        cstr!("//")
+        cstr!("//").into()
     );
     assert_eq!(
         SocketAddrUnix::new("/foo/bar").unwrap().path().unwrap(),
-        cstr!("/foo/bar")
+        cstr!("/foo/bar").into()
     );
     assert_eq!(
         SocketAddrUnix::new("foo").unwrap().path().unwrap(),
-        cstr!("foo")
+        cstr!("foo").into()
     );
     SocketAddrUnix::new("/foo\0/bar").unwrap_err();
     assert!(SocketAddrUnix::new("").unwrap().path().is_none());
diff --git a/tests/net/unix.rs b/tests/net/unix.rs
@@ -364,12 +364,22 @@ fn do_test_unix_msg_unconnected(addr: SocketAddrUnix) {
 #[cfg(not(any(target_os = "espidf", target_os = "redox", target_os = "wasi")))]
 #[test]
 fn test_unix_msg() {
+    use rustix::ffi::CString;
+    use std::os::unix::ffi::OsStrExt as _;
+
     crate::init();
 
     let tmpdir = tempfile::tempdir().unwrap();
     let path = tmpdir.path().join("scp_4804");
 
     let name = SocketAddrUnix::new(&path).unwrap();
+    assert_eq!(
+        name.path(),
+        Some(CString::new(path.as_os_str().as_bytes()).unwrap().into())
+    );
+    assert_eq!(name.path_bytes(), Some(path.as_os_str().as_bytes()));
+    #[cfg(linux_kernel)]
+    assert!(!name.is_unnamed());
     do_test_unix_msg(name);
 
     unlinkat(CWD, path, AtFlags::empty()).unwrap();
@@ -379,12 +389,22 @@ fn test_unix_msg() {
 #[cfg(not(any(target_os = "espidf", target_os = "redox", target_os = "wasi")))]
 #[test]
 fn test_unix_msg_unconnected() {
+    use rustix::ffi::CString;
+    use std::os::unix::ffi::OsStrExt as _;
+
     crate::init();
 
     let tmpdir = tempfile::tempdir().unwrap();
     let path = tmpdir.path().join("scp_4804");
 
     let name = SocketAddrUnix::new(&path).unwrap();
+    assert_eq!(
+        name.path(),
+        Some(CString::new(path.as_os_str().as_bytes()).unwrap().into())
+    );
+    assert_eq!(name.path_bytes(), Some(path.as_os_str().as_bytes()));
+    #[cfg(linux_kernel)]
+    assert!(!name.is_unnamed());
     do_test_unix_msg_unconnected(name);
 
     unlinkat(CWD, path, AtFlags::empty()).unwrap();
@@ -401,6 +421,8 @@ fn test_abstract_unix_msg() {
     let path = tmpdir.path().join("scp_4804");
 
     let name = SocketAddrUnix::new_abstract_name(path.as_os_str().as_bytes()).unwrap();
+    assert_eq!(name.abstract_name(), Some(path.as_os_str().as_bytes()));
+    assert!(!name.is_unnamed());
     do_test_unix_msg(name);
 }
 
@@ -416,6 +438,8 @@ fn test_abstract_unix_msg_unconnected() {
     let path = tmpdir.path().join("scp_4804");
 
     let name = SocketAddrUnix::new_abstract_name(path.as_os_str().as_bytes()).unwrap();
+    assert_eq!(name.abstract_name(), Some(path.as_os_str().as_bytes()));
+    assert!(!name.is_unnamed());
     do_test_unix_msg_unconnected(name);
 }
 
@@ -948,3 +972,35 @@ fn test_bind_unnamed_address() {
     assert_ne!(address.abstract_name(), None);
     assert_eq!(address.path(), None);
 }
+
+/// Test that names long enough to not have room for the NUL terminator are
+/// handled properly.
+#[test]
+fn test_long_named_address() {
+    use memoffset::span_of;
+    use rustix::ffi::CString;
+    use std::os::unix::ffi::OsStrExt;
+    use std::path::PathBuf;
+
+    let lens = [
+        span_of!(libc::sockaddr_un, sun_path).len(),
+        #[cfg(linux_kernel)]
+        span_of!(linux_raw_sys::net::sockaddr_un, sun_path).len(),
+    ];
+
+    for len in lens {
+        let path = PathBuf::from("a".repeat(len));
+        let name = SocketAddrUnix::new(&path).unwrap();
+        assert_eq!(
+            name.path(),
+            Some(CString::new(path.as_os_str().as_bytes()).unwrap().into())
+        );
+        assert_eq!(
+            name.path(),
+            Some(CString::new(path.as_os_str().as_bytes()).unwrap().into())
+        );
+        assert_eq!(name.path_bytes(), Some(path.as_os_str().as_bytes()));
+        #[cfg(linux_kernel)]
+        assert!(!name.is_unnamed());
+    }
+}
diff --git a/tests/net/unix_alloc.rs b/tests/net/unix_alloc.rs
