safe traversal: use the nix crate instead of unsafe calls to libc

This commit is contained in:
Sylvestre Ledru 2025-09-12 16:39:22 +02:00
parent 2bb097ba7b
commit 8fc888ebed
2 changed files with 99 additions and 168 deletions

View file

@ -80,7 +80,13 @@ fluent-bundle = { workspace = true }
thiserror = { workspace = true }
[target.'cfg(unix)'.dependencies]
walkdir = { workspace = true, optional = true }
nix = { workspace = true, features = ["fs", "uio", "zerocopy", "signal"] }
nix = { workspace = true, features = [
"fs",
"uio",
"zerocopy",
"signal",
"dir",
] }
xattr = { workspace = true, optional = true }
[dev-dependencies]

View file

@ -16,12 +16,17 @@
#[cfg(test)]
use std::os::unix::ffi::OsStringExt;
use std::ffi::{CStr, CString, OsStr, OsString};
use std::ffi::{CString, OsStr, OsString};
use std::io;
use std::os::unix::ffi::OsStrExt;
use std::os::unix::io::{AsRawFd, RawFd};
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd};
use std::path::Path;
use nix::dir::Dir;
use nix::fcntl::{OFlag, openat};
use nix::sys::stat::{FileStat, Mode, fstatat};
use nix::unistd::{UnlinkatFlags, unlinkat};
// Custom error types for better error reporting
#[derive(thiserror::Error, Debug)]
pub enum SafeTraversalError {
@ -71,87 +76,46 @@ impl From<SafeTraversalError> for io::Error {
}
}
// RAII wrapper for DIR pointer
struct Dir {
dirp: *mut libc::DIR,
}
// Helper function to read directory entries using nix
fn read_dir_entries(fd: &OwnedFd) -> io::Result<Vec<OsString>> {
let mut entries = Vec::new();
impl Dir {
fn from_fd(fd: RawFd) -> io::Result<Self> {
let dirp = unsafe { libc::fdopendir(fd) };
if dirp.is_null() {
Err(io::Error::last_os_error())
} else {
Ok(Dir { dirp })
// Duplicate the fd for Dir (it takes ownership)
let dup_fd = nix::unistd::dup(fd).map_err(|e| io::Error::from_raw_os_error(e as i32))?;
let mut dir = Dir::from_fd(dup_fd).map_err(|e| io::Error::from_raw_os_error(e as i32))?;
for entry_result in dir.iter() {
let entry = entry_result.map_err(|e| io::Error::from_raw_os_error(e as i32))?;
let name = entry.file_name();
let name_os = OsStr::from_bytes(name.to_bytes());
if name_os != "." && name_os != ".." {
entries.push(name_os.to_os_string());
}
}
fn read_entries(&self) -> io::Result<Vec<OsString>> {
let mut entries = Vec::new();
loop {
// Clear errno before readdir as per POSIX requirements
unsafe { *libc::__errno_location() = 0 };
let entry = unsafe { libc::readdir(self.dirp) };
if entry.is_null() {
let errno = unsafe { *libc::__errno_location() };
if errno != 0 {
return Err(io::Error::from_raw_os_error(errno));
}
break;
}
let name = unsafe { CStr::from_ptr((*entry).d_name.as_ptr()) };
let name_os = OsStr::from_bytes(name.to_bytes());
if name_os != "." && name_os != ".." {
entries.push(name_os.to_os_string());
}
}
Ok(entries)
}
}
impl Drop for Dir {
fn drop(&mut self) {
if !self.dirp.is_null() {
unsafe {
libc::closedir(self.dirp);
}
}
}
Ok(entries)
}
/// A directory file descriptor that enables safe traversal
pub struct DirFd {
fd: RawFd,
owned: bool,
fd: OwnedFd,
}
impl DirFd {
/// Open a directory and return a file descriptor
pub fn open(path: &Path) -> io::Result<Self> {
let path_cstr = CString::new(path.as_os_str().as_bytes())
.map_err(|_| SafeTraversalError::PathContainsNull)?;
let fd = unsafe {
libc::open(
path_cstr.as_ptr(),
libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
)
};
if fd < 0 {
Err(SafeTraversalError::OpenFailed {
let flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC;
let fd = nix::fcntl::open(path, flags, Mode::empty()).map_err(|e| {
SafeTraversalError::OpenFailed {
path: path.to_string_lossy().into_owned(),
source: io::Error::last_os_error(),
source: io::Error::from_raw_os_error(e as i32),
}
.into())
} else {
Ok(DirFd { fd, owned: true })
}
})?;
Ok(DirFd { fd })
}
/// Open a subdirectory relative to this directory
@ -159,48 +123,36 @@ impl DirFd {
let name_cstr =
CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?;
let fd = unsafe {
libc::openat(
self.fd,
name_cstr.as_ptr(),
libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
)
};
if fd < 0 {
Err(SafeTraversalError::OpenFailed {
let flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC;
let fd = openat(&self.fd, name_cstr.as_c_str(), flags, Mode::empty()).map_err(|e| {
SafeTraversalError::OpenFailed {
path: name.to_string_lossy().into_owned(),
source: io::Error::last_os_error(),
source: io::Error::from_raw_os_error(e as i32),
}
.into())
} else {
Ok(DirFd { fd, owned: true })
}
})?;
Ok(DirFd { fd })
}
/// Get raw stat data for a file relative to this directory
pub fn stat_at(&self, name: &OsStr, follow_symlinks: bool) -> io::Result<libc::stat> {
pub fn stat_at(&self, name: &OsStr, follow_symlinks: bool) -> io::Result<FileStat> {
let name_cstr =
CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?;
let mut stat: libc::stat = unsafe { std::mem::zeroed() };
let flags = if follow_symlinks {
0
nix::fcntl::AtFlags::empty()
} else {
libc::AT_SYMLINK_NOFOLLOW
nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW
};
let ret = unsafe { libc::fstatat(self.fd, name_cstr.as_ptr(), &mut stat, flags) };
if ret < 0 {
Err(SafeTraversalError::StatFailed {
let stat = fstatat(&self.fd, name_cstr.as_c_str(), flags).map_err(|e| {
SafeTraversalError::StatFailed {
path: name.to_string_lossy().into_owned(),
source: io::Error::last_os_error(),
source: io::Error::from_raw_os_error(e as i32),
}
.into())
} else {
Ok(stat)
}
})?;
Ok(stat)
}
/// Get metadata for a file relative to this directory
@ -214,43 +166,18 @@ impl DirFd {
}
/// Get raw stat data for this directory
pub fn fstat(&self) -> io::Result<libc::stat> {
let mut stat: libc::stat = unsafe { std::mem::zeroed() };
pub fn fstat(&self) -> io::Result<FileStat> {
let stat = nix::sys::stat::fstat(&self.fd).map_err(|e| SafeTraversalError::StatFailed {
path: "<current directory>".to_string(),
source: io::Error::from_raw_os_error(e as i32),
})?;
let ret = unsafe { libc::fstat(self.fd, &mut stat) };
if ret < 0 {
Err(SafeTraversalError::StatFailed {
path: "<current directory>".to_string(),
source: io::Error::last_os_error(),
}
.into())
} else {
Ok(stat)
}
Ok(stat)
}
/// Read directory entries
pub fn read_dir(&self) -> io::Result<Vec<OsString>> {
// Duplicate the fd for fdopendir (it takes ownership)
let dup_fd = unsafe { libc::dup(self.fd) };
if dup_fd < 0 {
return Err(SafeTraversalError::ReadDirFailed {
path: "<directory>".to_string(),
source: io::Error::last_os_error(),
}
.into());
}
let dir = Dir::from_fd(dup_fd).map_err(|e| {
unsafe { libc::close(dup_fd) };
SafeTraversalError::ReadDirFailed {
path: "<directory>".to_string(),
source: e,
}
})?;
dir.read_entries().map_err(|e| {
read_dir_entries(&self.fd).map_err(|e| {
SafeTraversalError::ReadDirFailed {
path: "<directory>".to_string(),
source: e,
@ -263,22 +190,23 @@ impl DirFd {
pub fn unlink_at(&self, name: &OsStr, is_dir: bool) -> io::Result<()> {
let name_cstr =
CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?;
let flags = if is_dir { libc::AT_REMOVEDIR } else { 0 };
let ret = unsafe { libc::unlinkat(self.fd, name_cstr.as_ptr(), flags) };
if ret < 0 {
Err(SafeTraversalError::UnlinkFailed {
path: name.to_string_lossy().into_owned(),
source: io::Error::last_os_error(),
}
.into())
let flags = if is_dir {
UnlinkatFlags::RemoveDir
} else {
Ok(())
}
UnlinkatFlags::NoRemoveDir
};
unlinkat(&self.fd, name_cstr.as_c_str(), flags).map_err(|e| {
SafeTraversalError::UnlinkFailed {
path: name.to_string_lossy().into_owned(),
source: io::Error::from_raw_os_error(e as i32),
}
})?;
Ok(())
}
/// Create a DirFd from an existing file descriptor (does not take ownership)
/// Create a DirFd from an existing file descriptor (takes ownership)
pub fn from_raw_fd(fd: RawFd) -> io::Result<Self> {
if fd < 0 {
return Err(io::Error::new(
@ -286,23 +214,21 @@ impl DirFd {
"invalid file descriptor",
));
}
Ok(DirFd { fd, owned: false })
}
}
impl Drop for DirFd {
fn drop(&mut self) {
if self.owned && self.fd >= 0 {
unsafe {
libc::close(self.fd);
}
}
// SAFETY: We've verified fd >= 0, and the caller is transferring ownership
let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) };
Ok(DirFd { fd: owned_fd })
}
}
impl AsRawFd for DirFd {
fn as_raw_fd(&self) -> RawFd {
self.fd
self.fd.as_raw_fd()
}
}
impl AsFd for DirFd {
fn as_fd(&self) -> BorrowedFd<'_> {
self.fd.as_fd()
}
}
@ -374,11 +300,11 @@ impl FileType {
/// Metadata wrapper for safer access to file information
#[derive(Debug, Clone)]
pub struct Metadata {
stat: libc::stat,
stat: FileStat,
}
impl Metadata {
pub fn from_stat(stat: libc::stat) -> Self {
pub fn from_stat(stat: FileStat) -> Self {
Self { stat }
}
@ -410,11 +336,6 @@ impl Metadata {
}
}
/// Get the raw libc::stat for compatibility with existing code
pub fn as_raw_stat(&self) -> &libc::stat {
&self.stat
}
/// Compatibility methods to match std::fs::Metadata interface
pub fn is_dir(&self) -> bool {
self.file_type().is_directory()
@ -558,6 +479,7 @@ mod tests {
use super::*;
use std::fs;
use std::os::unix::fs::symlink;
use std::os::unix::io::IntoRawFd;
use tempfile::TempDir;
#[test]
@ -696,11 +618,16 @@ mod tests {
fn test_from_raw_fd() {
let temp_dir = TempDir::new().unwrap();
let dir_fd = DirFd::open(temp_dir.path()).unwrap();
let raw_fd = dir_fd.as_raw_fd();
let borrowed_fd = DirFd::from_raw_fd(raw_fd).unwrap();
assert_eq!(borrowed_fd.as_raw_fd(), raw_fd);
assert!(!borrowed_fd.owned); // Should not own the FD
// Duplicate the fd first so we don't have ownership conflicts
let dup_fd = nix::unistd::dup(&dir_fd).unwrap();
let from_raw_fd = DirFd::from_raw_fd(dup_fd.into_raw_fd()).unwrap();
// Both should refer to the same directory
let stat1 = dir_fd.fstat().unwrap();
let stat2 = from_raw_fd.fstat().unwrap();
assert_eq!(stat1.st_ino, stat2.st_ino);
assert_eq!(stat1.st_dev, stat2.st_dev);
}
#[test]
@ -772,9 +699,7 @@ mod tests {
assert_eq!(metadata.mode() & libc::S_IFMT as u32, libc::S_IFREG as u32);
assert_eq!(metadata.nlink(), 1);
// Test raw stat access
let raw_stat = metadata.as_raw_stat();
assert_eq!(raw_stat.st_size, metadata.size() as i64);
assert!(metadata.size() > 0);
}
#[test]