From 10bd15aeaaf4be1d39c9e3a4b46a9d1c296c73d8 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Mon, 29 Jun 2026 01:28:41 -0400 Subject: [PATCH] fix(notify): harden chime temp-WAV writes against symlink-clobber (S5) cached_path materialized each embedded chime to a fixed, predictable path (/tmp/peerspeak-.wav) via fs::write, which follows symlinks -> a local attacker on a shared host could pre-plant a symlink and redirect the write. New write_private_wav seam writes to a randomized peerspeak---- .wav name with OpenOptions::create_new (O_EXCL, refuses to write through an existing path) and 0600 mode at creation on Unix. Per-process cache and the None-on-error fallback (chime simply doesn't play) are unchanged. Unit tests: exact bytes, 0600 mode, unique paths, create_new-refuses-existing. No new deps, no wire/schema change. Codex-implemented (gpt-5.5 xhigh), reviewed. Co-Authored-By: Claude Opus 4.8 --- src/notify.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 103 insertions(+), 5 deletions(-) diff --git a/src/notify.rs b/src/notify.rs index 58355d9..4475e79 100644 --- a/src/notify.rs +++ b/src/notify.rs @@ -11,12 +11,16 @@ //! missing chime should never disrupt a call. use std::collections::HashMap; +use std::fs::OpenOptions; +use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; +use std::time::{SystemTime, UNIX_EPOCH}; static ENABLED: AtomicBool = AtomicBool::new(true); +static TEMP_WAV_COUNTER: AtomicU64 = AtomicU64::new(0); /// Per-sound enable flags (W6), indexed by `Sound::index`. The master `ENABLED` /// toggle gates everything; these silence individual events while the master @@ -195,14 +199,38 @@ fn cached_path(sound: Sound) -> Option { if let Some(path) = guard.get(sound.name()) { return Some(path.clone()); } - let path = std::env::temp_dir().join(format!("peerspeak-{}.wav", sound.name())); - if std::fs::write(&path, sound.bytes()).is_err() { - return None; - } + let path = match write_private_wav(&std::env::temp_dir(), sound.name(), sound.bytes()) { + Ok(path) => path, + Err(_) => return None, + }; guard.insert(sound.name(), path.clone()); Some(path) } +fn write_private_wav(dir: &Path, stem: &str, bytes: &[u8]) -> std::io::Result { + let counter = TEMP_WAV_COUNTER.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + let path = dir.join(format!( + "peerspeak-{stem}-{}-{counter}-{nanos}.wav", + std::process::id() + )); + + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + + let mut file = options.open(&path)?; + file.write_all(bytes)?; + Ok(path) +} + #[cfg(any(windows, test))] fn escape_powershell_single_quoted(s: &str) -> String { s.replace('\'', "''") @@ -249,6 +277,19 @@ fn spawn_player(path: &Path) { mod tests { use super::*; + static TEST_TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); + + fn temp_wav_dir(tag: &str) -> PathBuf { + let counter = TEST_TEMP_COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "peerspeak-notifytest-{}-{tag}-{counter}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + #[test] fn test_should_play_truth_table() { // Plays only when BOTH the master and the per-sound flag are on. @@ -332,4 +373,61 @@ mod tests { // a `~`-prefixed path that resolves to a non-existent file -> Some(false) assert_eq!(validate_custom_path("~/non/existent/file.wav"), Some(false)); } + + #[test] + fn write_private_wav_writes_exact_bytes() { + let dir = temp_wav_dir("writes"); + let bytes = b"RIFFpeerspeak-test"; + + let path = write_private_wav(&dir, "unit", bytes).unwrap(); + + assert!(path.exists()); + assert_eq!(std::fs::read(&path).unwrap(), bytes); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn write_private_wav_creates_0600_file() { + use std::os::unix::fs::PermissionsExt; + + let dir = temp_wav_dir("mode"); + let path = write_private_wav(&dir, "unit", b"mode").unwrap(); + + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn write_private_wav_uses_unique_paths() { + let dir = temp_wav_dir("unique"); + + let first = write_private_wav(&dir, "same-stem", b"first").unwrap(); + let second = write_private_wav(&dir, "same-stem", b"second").unwrap(); + + assert_ne!(first, second); + assert!(first.exists()); + assert!(second.exists()); + assert_eq!(std::fs::read(&first).unwrap(), b"first"); + assert_eq!(std::fs::read(&second).unwrap(), b"second"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn create_new_refuses_existing_path() { + let dir = temp_wav_dir("create-new"); + let path = dir.join("preexisting.wav"); + std::fs::write(&path, b"original").unwrap(); + + let err = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .unwrap_err(); + + assert_eq!(err.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!(std::fs::read(&path).unwrap(), b"original"); + let _ = std::fs::remove_dir_all(&dir); + } }