From fe655be59a7b94829186f0e2363429488b9d981e Mon Sep 17 00:00:00 2001 From: Tarasov Aleksandr <55220741+arabianq@users.noreply.github.com> Date: Sun, 17 May 2026 17:43:51 +0300 Subject: [PATCH] fix: insecure fallback directory and secure file creation (#111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔒 Fix insecure fallback directory and secure file creation The daemon's fallback runtime directory `get_runtime_dir()` was hardcoded to `/run/pwsp`, creating a risk of shared, insecure access in multi-user systems. This commit secures the fallback logic by: 1. Creating a user-specific temporary directory (`/tmp/pwsp-$UID`). 2. Ensuring directory creation happens atomically with `0o700` permissions using `std::fs::DirBuilder`. 3. Validating the fallback directory strictly (checking UID, 0o700 permissions, and symlink status) if it already exists to mitigate symlink attacks. 4. Using `libc::geteuid()` for robust cross-platform UID extraction. 5. Fixing `is_daemon_running` and locking logic to use `fs::OpenOptions` instead of `fs::File::create` to prevent accidental file truncation on active lock files. Co-authored-by: arabianq <55220741+arabianq@users.noreply.github.com> * 🔒 Fix insecure fallback directory and secure file creation The daemon's fallback runtime directory `get_runtime_dir()` was hardcoded to `/run/pwsp`, creating a risk of shared, insecure access in multi-user systems. This commit secures the fallback logic by: 1. Creating a user-specific temporary directory (`/tmp/pwsp-$UID`). 2. Ensuring directory creation happens atomically with `0o700` permissions using `std::fs::DirBuilder`. 3. Validating the fallback directory strictly (checking UID, 0o700 permissions, and symlink status) if it already exists to mitigate symlink attacks. 4. Using safe `rustix::process::geteuid()` for robust cross-platform UID extraction, avoiding `unsafe` blocks. 5. Fixing `is_daemon_running` and locking logic to use `fs::OpenOptions` instead of `fs::File::create` to prevent accidental file truncation on active lock files. Co-authored-by: arabianq <55220741+arabianq@users.noreply.github.com> * small refactor --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --- Cargo.lock | 1 + Cargo.toml | 1 + src/bin/daemon.rs | 6 +++++- src/utils/daemon.rs | 44 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 11fd40b..ade2ce5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3177,6 +3177,7 @@ dependencies = [ "rfd", "rodio", "rust-i18n", + "rustix 1.1.4", "serde", "serde_json", "sys-locale", diff --git a/Cargo.toml b/Cargo.toml index 4b619aa..e60dcb8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ rfd = { version = "0.17.2", default-features = false, features = [ opener = { version = "0.8.4", features = ["reveal"] } system-fonts = "0.1.0" anyhow = "1.0.102" +rustix = { version = "1.1.4", features = ["process"] } rust-i18n = "4.0.0" sys-locale = "0.3.2" diff --git a/src/bin/daemon.rs b/src/bin/daemon.rs index 78e17ba..e6eb153 100644 --- a/src/bin/daemon.rs +++ b/src/bin/daemon.rs @@ -39,7 +39,11 @@ async fn main() -> Result<()> { let runtime_dir = get_runtime_dir(); - let lock_file = fs::File::create(runtime_dir.join("daemon.lock"))?; + let lock_file = fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .open(runtime_dir.join("daemon.lock"))?; lock_file.lock()?; let socket_path = runtime_dir.join("daemon.sock"); diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index 864d155..45e4538 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -5,9 +5,9 @@ use crate::types::{ }; use anyhow::Result; -use std::os::unix::fs::PermissionsExt; +use std::os::unix::fs::{DirBuilderExt, MetadataExt, PermissionsExt}; use std::path::PathBuf; -use std::{error::Error, fs}; +use std::{env, error::Error, fs}; use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, net::UnixStream, @@ -37,22 +37,52 @@ pub fn get_daemon_config() -> DaemonConfig { }) } +fn get_current_uid() -> u32 { + rustix::process::geteuid().as_raw() +} + pub fn get_runtime_dir() -> PathBuf { - dirs::runtime_dir().unwrap_or(PathBuf::from("/run/pwsp")) + dirs::runtime_dir().unwrap_or_else(|| { + let uid = get_current_uid(); + env::temp_dir().join(format!("pwsp-{}", uid)) + }) } pub fn create_runtime_dir() -> Result<()> { let runtime_dir = get_runtime_dir(); - if !runtime_dir.exists() { - fs::create_dir_all(&runtime_dir)?; + + if runtime_dir.exists() { + let meta = fs::symlink_metadata(&runtime_dir)?; + if meta.is_symlink() { + return Err(anyhow::anyhow!("Runtime directory is a symlink")); + } + let uid = get_current_uid(); + if meta.uid() != uid { + return Err(anyhow::anyhow!( + "Runtime directory is owned by another user" + )); + } + if meta.permissions().mode() & 0o777 != 0o700 { + return Err(anyhow::anyhow!( + "Runtime directory has incorrect permissions" + )); + } + } else { + fs::DirBuilder::new() + .recursive(true) + .mode(0o700) + .create(&runtime_dir)?; } - fs::set_permissions(&runtime_dir, fs::Permissions::from_mode(0o700))?; Ok(()) } pub fn is_daemon_running() -> Result { - let lock_file = fs::File::create(get_runtime_dir().join("daemon.lock"))?; + let lock_file = fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .open(get_runtime_dir().join("daemon.lock"))?; match lock_file.try_lock() { Ok(_) => Ok(false), Err(_) => Ok(true),