Only accept UTF-8 directory entries

native/src/base/dir.rs

@@ -1,10 +1,9 @@
 use crate::cxx_extern::readlinkat;
 use crate::{
     FileAttr, FsPathBuf, LibcReturn, OsError, OsResult, OsResultStatic, Utf8CStr, Utf8CStrBuf,
-    cstr, cstr_buf, errno, fd_path, fd_set_attr,
+    cstr_buf, errno, fd_path, fd_set_attr,
 };
 use libc::{EEXIST, O_CLOEXEC, O_CREAT, O_RDONLY, O_TRUNC, O_WRONLY, dirent, mode_t};
-use std::ffi::CStr;
 use std::fs::File;
 use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
@@ -23,20 +22,15 @@ impl DirEntry<'_> {
         self.entry.as_ptr()
     }
 
-    pub fn name(&self) -> &CStr {
+    pub fn name(&self) -> &Utf8CStr {
         unsafe {
-            CStr::from_bytes_with_nul_unchecked(slice::from_raw_parts(
+            Utf8CStr::from_bytes_unchecked(slice::from_raw_parts(
                 self.d_name.as_ptr().cast(),
                 self.d_name_len,
             ))
         }
     }
 
-    #[inline(always)]
-    fn utf8_name(&self) -> Option<&str> {
-        self.name().to_str().ok()
-    }
-
     pub fn path(&self, buf: &mut dyn Utf8CStrBuf) -> OsResult<'static, ()> {
         self.dir.path_at(self.name(), buf)
     }
@@ -74,7 +68,7 @@ impl DirEntry<'_> {
         unsafe {
             libc::unlinkat(self.dir.as_raw_fd(), self.d_name.as_ptr(), flag).check_os_err(
                 "unlinkat",
-                self.utf8_name(),
+                Some(self.name()),
                 None,
             )?;
         }
@@ -90,7 +84,7 @@ impl DirEntry<'_> {
                 buf.as_mut_ptr().cast(),
                 buf.capacity(),
             )
-            .as_os_result("readlinkat", self.utf8_name(), None)? as usize;
+            .as_os_result("readlinkat", Some(self.name()), None)? as usize;
             buf.set_len(r);
         }
         Ok(())
@@ -101,7 +95,7 @@ impl DirEntry<'_> {
             return Err(OsError::with_os_error(
                 libc::ENOTDIR,
                 "fdopendir",
-                self.utf8_name(),
+                Some(self.name()),
                 None,
             ));
         }
@@ -113,7 +107,7 @@ impl DirEntry<'_> {
             return Err(OsError::with_os_error(
                 libc::EISDIR,
                 "open_as_file",
-                self.utf8_name(),
+                Some(self.name()),
                 None,
             ));
         }
@@ -180,22 +174,17 @@ impl Directory {
         }
     }
 
-    fn openat<'a>(&self, name: &'a CStr, flags: i32, mode: u32) -> OsResult<'a, OwnedFd> {
+    fn openat<'a>(&self, name: &'a Utf8CStr, flags: i32, mode: u32) -> OsResult<'a, OwnedFd> {
         unsafe {
             libc::openat(self.as_raw_fd(), name.as_ptr(), flags | O_CLOEXEC, mode)
-                .as_os_result("openat", name.to_str().ok(), None)
+                .as_os_result("openat", Some(name), None)
                 .map(|fd| OwnedFd::from_raw_fd(fd))
         }
     }
 
-    fn path_at(&self, name: &CStr, buf: &mut dyn Utf8CStrBuf) -> OsResult<'static, ()> {
+    fn path_at(&self, name: &Utf8CStr, buf: &mut dyn Utf8CStrBuf) -> OsResult<'static, ()> {
         self.path(buf)?;
-        if let Ok(s) = name.to_str() {
+        FsPathBuf::from(buf).join(name);
-            FsPathBuf::from(buf).join(s);
-        } else {
-            buf.push_str("/");
-            buf.push_lossy(name.to_bytes());
-        }
         Ok(())
     }
 }
@@ -217,17 +206,21 @@ impl Directory {
                 Ok(None)
             };
         }
-        // Skip both "." and ".."
+        // Skip non UTF-8 entries, ".", and ".."
         unsafe {
             let entry = &*e;
-            let d_name = CStr::from_ptr(entry.d_name.as_ptr());
+
-            if d_name == cstr!(".") || d_name == cstr!("..") {
+            let Ok(name) = Utf8CStr::from_ptr(entry.d_name.as_ptr()) else {
+                return self.read();
+            };
+
+            if name == "." || name == ".." {
                 self.read()
             } else {
                 let e = DirEntry {
                     dir: self.borrow(),
                     entry: NonNull::from(entry),
-                    d_name_len: d_name.to_bytes_with_nul().len(),
+                    d_name_len: name.as_bytes_with_nul().len(),
                 };
                 Ok(Some(e))
             }
@@ -238,60 +231,64 @@ impl Directory {
         unsafe { libc::rewinddir(self.inner.as_ptr()) };
     }
 
-    pub fn openat_as_dir<'a>(&self, name: &'a CStr) -> OsResult<'a, Directory> {
+    pub fn openat_as_dir<'a>(&self, name: &'a Utf8CStr) -> OsResult<'a, Directory> {
         let fd = self.openat(name, O_RDONLY, 0)?;
-        Directory::try_from(fd).map_err(|e| e.set_args(name.to_str().ok(), None))
+        Directory::try_from(fd).map_err(|e| e.set_args(Some(name), None))
     }
 
-    pub fn openat_as_file<'a>(&self, name: &'a CStr, flags: i32, mode: u32) -> OsResult<'a, File> {
+    pub fn openat_as_file<'a>(
+        &self,
+        name: &'a Utf8CStr,
+        flags: i32,
+        mode: u32,
+    ) -> OsResult<'a, File> {
         let fd = self.openat(name, flags, mode)?;
         Ok(File::from(fd))
     }
 
-    pub fn get_attr_at<'a>(&self, name: &'a CStr) -> OsResult<'a, FileAttr> {
+    pub fn get_attr_at<'a>(&self, name: &'a Utf8CStr) -> OsResult<'a, FileAttr> {
         let mut path = FsPathBuf::default();
         self.path_at(name, path.0.deref_mut())?;
-        path.get_attr()
+        path.get_attr().map_err(|e| e.set_args(Some(name), None))
-            .map_err(|e| e.set_args(name.to_str().ok(), None))
     }
 
-    pub fn set_attr_at<'a>(&self, name: &'a CStr, attr: &FileAttr) -> OsResult<'a, ()> {
+    pub fn set_attr_at<'a>(&self, name: &'a Utf8CStr, attr: &FileAttr) -> OsResult<'a, ()> {
         let mut path = FsPathBuf::default();
         self.path_at(name, path.0.deref_mut())?;
         path.set_attr(attr)
-            .map_err(|e| e.set_args(name.to_str().ok(), None))
+            .map_err(|e| e.set_args(Some(name), None))
     }
 
     pub fn get_secontext_at<'a>(
         &self,
-        name: &'a CStr,
+        name: &'a Utf8CStr,
         con: &mut dyn Utf8CStrBuf,
     ) -> OsResult<'a, ()> {
         let mut path = FsPathBuf::default();
         self.path_at(name, path.0.deref_mut())?;
         path.get_secontext(con)
-            .map_err(|e| e.set_args(name.to_str().ok(), None))
+            .map_err(|e| e.set_args(Some(name), None))
     }
 
-    pub fn set_secontext_at<'a>(&self, name: &'a CStr, con: &'a Utf8CStr) -> OsResult<'a, ()> {
+    pub fn set_secontext_at<'a>(&self, name: &'a Utf8CStr, con: &'a Utf8CStr) -> OsResult<'a, ()> {
         let mut path = FsPathBuf::default();
         self.path_at(name, path.0.deref_mut())?;
         path.set_secontext(con)
-            .map_err(|e| e.set_args(name.to_str().ok(), Some(con)))
+            .map_err(|e| e.set_args(Some(name), Some(con)))
     }
 
-    pub fn mkdirat<'a>(&self, name: &'a CStr, mode: mode_t) -> OsResult<'a, ()> {
+    pub fn mkdirat<'a>(&self, name: &'a Utf8CStr, mode: mode_t) -> OsResult<'a, ()> {
         unsafe {
             if libc::mkdirat(self.as_raw_fd(), name.as_ptr(), mode as mode_t) < 0
                 && *errno() != EEXIST
             {
-                return Err(OsError::last_os_error("mkdirat", name.to_str().ok(), None));
+                return Err(OsError::last_os_error("mkdirat", Some(name), None));
             }
         }
         Ok(())
     }
 
-    pub fn contains_path(&self, path: &CStr) -> bool {
+    pub fn contains_path(&self, path: &Utf8CStr) -> bool {
         // WARNING: Using faccessat is incorrect, because the raw linux kernel syscall
         // does not support the flag AT_SYMLINK_NOFOLLOW until 5.8 with faccessat2.
         // Use fstatat to check the existence of a file instead.
@@ -351,7 +348,7 @@ impl Directory {
                 e.read_link(&mut target)?;
                 unsafe {
                     libc::symlinkat(target.as_ptr(), dir.as_raw_fd(), e.d_name.as_ptr())
-                        .check_os_err("symlinkat", Some(&target), e.utf8_name())?;
+                        .check_os_err("symlinkat", Some(&target), Some(e.name()))?;
                 }
                 dir.set_attr_at(e.name(), &attr)?;
             }
@@ -377,7 +374,7 @@ impl Directory {
                     dir.as_raw_fd(),
                     e.d_name.as_ptr(),
                 )
-                .check_os_err("renameat", e.utf8_name(), None)?;
+                .check_os_err("renameat", Some(e.name()), None)?;
             }
         }
         Ok(())
@@ -402,7 +399,7 @@ impl Directory {
                         e.d_name.as_ptr(),
                         0,
                     )
-                    .check_os_err("linkat", e.utf8_name(), None)?;
+                    .check_os_err("linkat", Some(e.name()), None)?;
                 }
             }
         }

native/src/core/lib.rs

@@ -16,11 +16,11 @@ use logging::{android_logging, setup_logfile, zygisk_close_logd, zygisk_get_logd
 use mount::{find_preinit_device, revert_unmount};
 use resetprop::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop};
 use socket::{recv_fd, recv_fds, send_fd, send_fds};
-use su::{pump_tty, get_pty_num, restore_stdin};
 use std::fs::File;
 use std::mem::ManuallyDrop;
 use std::ops::DerefMut;
 use std::os::fd::FromRawFd;
+use su::{get_pty_num, pump_tty, restore_stdin};
 use zygisk::zygisk_should_load_module;
 
 #[path = "../include/consts.rs"]

native/src/core/package.rs

@@ -157,7 +157,7 @@ fn find_apk_path(pkg: &str, buf: &mut dyn Utf8CStrBuf) -> LoggedResult<()> {
         if !e.is_dir() {
             return Ok(Skip);
         }
-        let name_bytes = e.name().to_bytes();
+        let name_bytes = e.name().as_bytes();
         if name_bytes.starts_with(pkg.as_bytes()) && name_bytes[pkg.len()] == b'-' {
             // Found the APK path, we can abort now
             e.path(buf)?;

native/src/core/resetprop/persist.rs

@@ -164,10 +164,8 @@ pub fn persist_get_props(mut prop_cb: Pin<&mut PropCb>) {
             let mut dir = Directory::open(cstr!(PERSIST_PROP_DIR))?;
             dir.pre_order_walk(|e| {
                 if e.is_file() {
-                    if let Ok(name) = Utf8CStr::from_cstr(e.name()) {
+                    if let Ok(mut value) = file_get_prop(e.name()) {
-                        if let Ok(mut value) = file_get_prop(name) {
+                        prop_cb.exec(e.name(), Utf8CStr::from_string(&mut value));
-                            prop_cb.exec(name, Utf8CStr::from_string(&mut value));
-                        }
                     }
                 }
                 // Do not traverse recursively

native/src/core/su/mod.rs

@@ -3,4 +3,4 @@ mod db;
 mod pts;
 
 pub use daemon::SuInfo;
-pub use pts::{pump_tty, get_pty_num, restore_stdin};
+pub use pts::{get_pty_num, pump_tty, restore_stdin};

native/src/core/zygisk/daemon.rs

@@ -191,7 +191,7 @@ impl MagiskD {
                     .join("zygisk");
                 // Create the unloaded marker file
                 if let Ok(dir) = Directory::open(&path) {
-                    dir.openat_as_file(cstr!("unloaded").as_cstr(), O_CREAT | O_RDONLY, 0o644)
+                    dir.openat_as_file(cstr!("unloaded"), O_CREAT | O_RDONLY, 0o644)
                         .log()
                         .ok();
                 }

native/src/init/rootdir.rs

@@ -2,8 +2,8 @@ use crate::consts::{ROOTMNT, ROOTOVL};
 use crate::ffi::MagiskInit;
 use base::libc::{O_CREAT, O_RDONLY, O_WRONLY};
 use base::{
-    BufReadExt, Directory, FsPath, FsPathBuf, LoggedResult, ResultExt, Utf8CStr,
+    BufReadExt, Directory, FsPath, FsPathBuf, LoggedResult, ResultExt, Utf8CStr, Utf8CString,
-    Utf8CString, clone_attr, cstr, cstr_buf, debug, path,
+    clone_attr, cstr, cstr_buf, debug, path,
 };
 use std::io::BufReader;
 use std::{
@@ -71,7 +71,7 @@ impl MagiskInit {
             match &dir.read()? {
                 None => return Ok(()),
                 Some(e) => {
-                    let name = e.name().to_str()?;
+                    let name = e.name();
                     let src = FsPathBuf::from(cstr_buf::dynamic(256))
                         .join(src_dir)
                         .join(name);