From 1f0a7e76979c16bd6ace8f9dbbf59053e98300f5 Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Tue, 20 Jan 2026 18:19:51 +0100 Subject: [PATCH] nix: use wrappers for `geteuid` and `getegid` The nix crate offers thin wrappers around these functions which allow us to get rid of our own libc wrappers, reducing the amount of code marked `unsafe`. Part of #12380 --- Cargo.toml | 1 + src/builtins/path.rs | 12 ++++++------ src/builtins/test.rs | 6 ++++-- src/env/environment.rs | 21 +++++++++++++-------- src/nix.rs | 6 ------ 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6faadea71..43bfda7ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ nix = { version = "0.30.1", default-features = false, features = [ "resource", "signal", "term", + "user", ] } num-traits = "0.2.19" once_cell = "1.19.0" diff --git a/src/builtins/path.rs b/src/builtins/path.rs index 95fc64d12..754285bea 100644 --- a/src/builtins/path.rs +++ b/src/builtins/path.rs @@ -4,7 +4,6 @@ use std::time::SystemTime; use super::prelude::*; -use crate::nix::{getegid, geteuid}; use crate::path::path_apply_working_directory; use crate::wutil::{ INVALID_FILE_ID, file_id_for_path, lwstat, normalize_path, waccess, wbasename, wdirname, @@ -14,6 +13,7 @@ use fish_util::wcsfilecmp_glob; use fish_wcstringutil::split_string_tok; use libc::{F_OK, PATH_MAX, R_OK, S_ISGID, S_ISUID, W_OK, X_OK, mode_t}; +use nix::unistd::{Gid, Uid}; macro_rules! path_error { ( @@ -743,7 +743,7 @@ fn path_sort(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Bu Ok(SUCCESS) } -fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) -> bool { +fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) -> bool { // TODO: Add moar stuff: // fifos, sockets, size greater than zero, setuid, ... // Nothing to check, file existence is checked elsewhere. @@ -832,9 +832,9 @@ fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) return false; } else if perm.contains(PermFlags::SGID) && (md.mode() as mode_t & S_ISGID) == 0 { return false; - } else if perm.contains(PermFlags::USER) && uid != Some(md.uid()) { + } else if perm.contains(PermFlags::USER) && uid.map(|u| u.as_raw()) != Some(md.uid()) { return false; - } else if perm.contains(PermFlags::GROUP) && gid != Some(md.gid()) { + } else if perm.contains(PermFlags::GROUP) && gid.map(|g| g.as_raw()) != Some(md.gid()) { return false; } } @@ -874,12 +874,12 @@ fn path_filter_maybe_is( // If we're looking for the owner/group, get our euid/egid here once. let uid = if opts.perms.unwrap_or_default().contains(PermFlags::USER) { - Some(geteuid()) + Some(Uid::effective()) } else { None }; let gid = if opts.perms.unwrap_or_default().contains(PermFlags::GROUP) { - Some(getegid()) + Some(Gid::effective()) } else { None }; diff --git a/src/builtins/test.rs b/src/builtins/test.rs index f971b8cc4..0e04b3264 100644 --- a/src/builtins/test.rs +++ b/src/builtins/test.rs @@ -3,6 +3,8 @@ use crate::should_flog; mod test_expressions { + use nix::unistd::{Gid, Uid}; + use super::*; use crate::nix::isatty; @@ -931,13 +933,13 @@ fn unary_primary_evaluate( // "-f", for regular files StatPredicate::f => md.file_type().is_file(), // "-G", for check effective group ID - StatPredicate::G => md.gid() == crate::nix::getegid(), + StatPredicate::G => md.gid() == Gid::effective().as_raw(), // "-g", for set-group-id StatPredicate::g => md.permissions().mode() & S_ISGID != 0, // "-k", for sticky bit StatPredicate::k => md.permissions().mode() & S_ISVTX != 0, // "-O", for check effective user id - StatPredicate::O => md.uid() == crate::nix::geteuid(), + StatPredicate::O => md.uid() == Uid::effective().as_raw(), // "-p", for FIFO StatPredicate::p => md.file_type().is_fifo(), // "-S", socket diff --git a/src/env/environment.rs b/src/env/environment.rs index 920447aad..78a5259ba 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -16,7 +16,7 @@ use crate::global_safety::RelaxedAtomicBool; use crate::input::{FISH_BIND_MODE_VAR, init_input}; use crate::localization::wgettext; -use crate::nix::{geteuid, getpid}; +use crate::nix::getpid; use crate::null_terminated_array::OwningNullTerminatedArray; use crate::path::{ path_emit_config_directory_messages, path_get_cache, path_get_config, path_get_data, @@ -29,7 +29,8 @@ use crate::wutil::{fish_wcstol, wgetcwd}; use fish_wcstringutil::join_strings; -use libc::{c_int, uid_t}; +use libc::c_int; +use nix::unistd::Uid; use std::collections::HashMap; use std::ffi::CStr; use std::mem::MaybeUninit; @@ -480,7 +481,7 @@ fn get_hostname_identifier() -> Option { /// Get values for $HOME via getpwuid, /// without trusting $USER or $HOME. pub fn get_home() -> Option { - let uid: uid_t = geteuid(); + let uid = Uid::effective(); let mut userinfo: MaybeUninit = MaybeUninit::uninit(); let mut result: *mut libc::passwd = std::ptr::null_mut(); @@ -489,7 +490,7 @@ pub fn get_home() -> Option { // We need to get the data via the uid and don't trust $USER. let retval = unsafe { libc::getpwuid_r( - uid, + uid.as_raw(), userinfo.as_mut_ptr(), buf.as_mut_ptr(), buf.len(), @@ -512,7 +513,7 @@ pub fn get_home() -> Option { /// Set up the USER and HOME variable. fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { - let uid: uid_t = geteuid(); + let uid = Uid::effective(); let user_var = vars.get_unless_empty(L!("USER")); let mut userinfo: MaybeUninit = MaybeUninit::uninit(); @@ -534,7 +535,7 @@ fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { }; if retval == 0 && !result.is_null() { let userinfo = unsafe { userinfo.assume_init() }; - if unsafe { *result }.pw_uid == uid { + if unsafe { *result }.pw_uid == uid.as_raw() { // The uid matches but we still might need to set $HOME. if vars.get_unless_empty(L!("HOME")).is_none() { if !userinfo.pw_dir.is_null() { @@ -553,7 +554,7 @@ fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { // We need to get the data *again* via the uid. let retval = unsafe { libc::getpwuid_r( - uid, + uid.as_raw(), userinfo.as_mut_ptr(), buf.as_mut_ptr(), buf.len(), @@ -655,7 +656,11 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // Set $USER, $HOME and $EUID // This involves going to passwd and stuff. - vars.set_one(L!("EUID"), global_mode, geteuid().to_wstring()); + vars.set_one( + L!("EUID"), + global_mode, + Uid::effective().as_raw().to_wstring(), + ); setup_user(global_exported_mode, vars); if let Some(paths) = paths { diff --git a/src/nix.rs b/src/nix.rs index 18712ca29..b86bd8a69 100644 --- a/src/nix.rs +++ b/src/nix.rs @@ -26,12 +26,6 @@ fn as_duration(&self) -> Duration { } } -pub fn geteuid() -> u32 { - unsafe { libc::geteuid() } -} -pub fn getegid() -> u32 { - unsafe { libc::getegid() } -} pub fn getpgrp() -> i32 { unsafe { libc::getpgrp() } }