From 7a207d4ccfaa8525183a51b1cead44159f907ae4 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Tue, 15 Apr 2025 00:19:28 -0700 Subject: [PATCH] Only accept UTF-8 directory entries --- native/src/base/dir.rs | 85 ++++++++++++++-------------- native/src/core/lib.rs | 2 +- native/src/core/package.rs | 2 +- native/src/core/resetprop/persist.rs | 6 +- native/src/core/su/mod.rs | 2 +- native/src/core/zygisk/daemon.rs | 2 +- native/src/init/rootdir.rs | 6 +- 7 files changed, 50 insertions(+), 55 deletions(-) diff --git a/native/src/base/dir.rs b/native/src/base/dir.rs index 7c2bbb38d..d2f8102b0 100644 --- a/native/src/base/dir.rs +++ b/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(s); - } else { - buf.push_str("/"); - buf.push_lossy(name.to_bytes()); - } + FsPathBuf::from(buf).join(name); 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() - .map_err(|e| e.set_args(name.to_str().ok(), None)) + path.get_attr().map_err(|e| e.set_args(Some(name), 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)?; } } } diff --git a/native/src/core/lib.rs b/native/src/core/lib.rs index 1fdbb9cbd..1d132bc10 100644 --- a/native/src/core/lib.rs +++ b/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"] diff --git a/native/src/core/package.rs b/native/src/core/package.rs index 220655634..f75b3990a 100644 --- a/native/src/core/package.rs +++ b/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)?; diff --git a/native/src/core/resetprop/persist.rs b/native/src/core/resetprop/persist.rs index c054b7ec2..7dafd6083 100644 --- a/native/src/core/resetprop/persist.rs +++ b/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(name) { - prop_cb.exec(name, Utf8CStr::from_string(&mut value)); - } + if let Ok(mut value) = file_get_prop(e.name()) { + prop_cb.exec(e.name(), Utf8CStr::from_string(&mut value)); } } // Do not traverse recursively diff --git a/native/src/core/su/mod.rs b/native/src/core/su/mod.rs index 59fafe849..cea1de87e 100644 --- a/native/src/core/su/mod.rs +++ b/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}; diff --git a/native/src/core/zygisk/daemon.rs b/native/src/core/zygisk/daemon.rs index 3cbac1c0b..43a730a94 100644 --- a/native/src/core/zygisk/daemon.rs +++ b/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(); } diff --git a/native/src/init/rootdir.rs b/native/src/init/rootdir.rs index 796fc5697..86e744c1b 100644 --- a/native/src/init/rootdir.rs +++ b/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, - Utf8CString, clone_attr, cstr, cstr_buf, debug, path, + BufReadExt, Directory, FsPath, FsPathBuf, LoggedResult, ResultExt, Utf8CStr, Utf8CString, + 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);