diff --git a/server_manager/src/core/atomic_io.rs b/server_manager/src/core/atomic_io.rs index 07e2030..fde1718 100644 --- a/server_manager/src/core/atomic_io.rs +++ b/server_manager/src/core/atomic_io.rs @@ -15,6 +15,7 @@ use std::os::unix::fs::OpenOptionsExt; /// 4. Synchronizes to disk via `fsync` (`sync_all`). /// 5. Atomically renames the temporary file to the destination path. pub fn atomic_write>(path: P, content: &[u8], mode: u32) -> Result<()> { + crate::core::validate::validate_safe_path(&path)?; let dest = path.as_ref(); let parent = dest.parent().unwrap_or_else(|| Path::new(".")); if !parent.as_os_str().is_empty() { diff --git a/server_manager/src/core/journal.rs b/server_manager/src/core/journal.rs index e4adc45..512a62e 100644 --- a/server_manager/src/core/journal.rs +++ b/server_manager/src/core/journal.rs @@ -66,6 +66,7 @@ impl Journal { /// Opens or creates the journal file with 0600 permissions. pub fn open_or_create>(path: P) -> Result { + crate::core::validate::validate_safe_path(&path)?; let target = path.as_ref(); let parent = target.parent().unwrap_or_else(|| Path::new(".")); if !parent.as_os_str().is_empty() { diff --git a/server_manager/src/core/lock.rs b/server_manager/src/core/lock.rs index fb3e8cd..c11171d 100644 --- a/server_manager/src/core/lock.rs +++ b/server_manager/src/core/lock.rs @@ -15,6 +15,7 @@ impl ProcessLock { /// Attempts to acquire an exclusive lock on the specified path. /// If `non_blocking` is true and the lock is already held, returns an error immediately. pub fn acquire>(path: P, non_blocking: bool) -> Result { + crate::core::validate::validate_safe_path(&path)?; let target = path.as_ref(); let parent = target.parent().unwrap_or_else(|| Path::new(".")); if !parent.as_os_str().is_empty() { diff --git a/server_manager/src/core/validate.rs b/server_manager/src/core/validate.rs index 9c4898e..5233e8c 100644 --- a/server_manager/src/core/validate.rs +++ b/server_manager/src/core/validate.rs @@ -139,10 +139,25 @@ pub fn validate_ip(ip_str: &str) -> Result { .map_err(|_| anyhow::anyhow!("Validation error: invalid IP address '{}'", ip_str)) } -/// Validates that a path does not contain directory traversal sequences (`..`). +/// Validates that a path does not contain directory traversal sequences (`..`), NUL bytes, or control characters. pub fn validate_safe_path>(path: P) -> Result

{ let p = path.as_ref(); - let normalized = p.to_string_lossy().replace('\\', "/"); + let path_str = p.to_string_lossy(); + if path_str.contains('\0') { + bail!( + "Validation error: NUL byte forbidden in path '{}'", + p.display() + ); + } + for c in path_str.chars() { + if c.is_ascii_control() { + bail!( + "Validation error: control character forbidden in path '{}'", + p.display() + ); + } + } + let normalized = path_str.replace('\\', "/"); let norm_path = Path::new(&normalized); for comp in norm_path.components() { if comp == Component::ParentDir { diff --git a/server_manager/tests/contract_atomic_io.rs b/server_manager/tests/contract_atomic_io.rs index 1c96f20..1ff8900 100644 --- a/server_manager/tests/contract_atomic_io.rs +++ b/server_manager/tests/contract_atomic_io.rs @@ -31,6 +31,12 @@ fn test_atomic_write_creates_file_with_content() { let _ = fs::remove_dir_all(&temp_dir); } +#[test] +fn test_atomic_write_rejects_unsafe_path() { + let bytes = b"test payload"; + assert!(atomic_write("../unsafe_atomic_write.tmp", bytes, 0o600).is_err()); +} + #[test] fn test_atomic_write_overwrites_existing_file() { let temp_dir = std::env::temp_dir().join(format!( diff --git a/server_manager/tests/contract_input_validation.rs b/server_manager/tests/contract_input_validation.rs index 7b9da2d..e7ab059 100644 --- a/server_manager/tests/contract_input_validation.rs +++ b/server_manager/tests/contract_input_validation.rs @@ -120,4 +120,10 @@ fn test_validate_safe_path_extended_traversal() { assert!(validate_safe_path(Path::new("..\\secret")).is_err()); assert!(validate_safe_path(Path::new("..")).is_err()); assert!(validate_safe_path(Path::new(".")).is_ok()); + + // NUL byte & control characters + assert!(validate_safe_path(Path::new("config.yaml\0.bak")).is_err()); + assert!(validate_safe_path(Path::new("config\nfile.txt")).is_err()); + assert!(validate_safe_path(Path::new("config\rfile.txt")).is_err()); + assert!(validate_safe_path(Path::new("config\tfile.txt")).is_err()); } diff --git a/server_manager/tests/contract_journal.rs b/server_manager/tests/contract_journal.rs index 6ed31d8..1691380 100644 --- a/server_manager/tests/contract_journal.rs +++ b/server_manager/tests/contract_journal.rs @@ -45,6 +45,11 @@ fn test_journal_creation_and_append() { let _ = fs::remove_dir_all(&temp_dir); } +#[test] +fn test_journal_open_or_create_rejects_unsafe_path() { + assert!(Journal::open_or_create("../unsafe_journal.jsonl").is_err()); +} + #[test] fn test_journal_compensatory_rollback_in_reverse_order() { let temp_dir = std::env::temp_dir().join(format!( diff --git a/server_manager/tests/contract_locking.rs b/server_manager/tests/contract_locking.rs index 49bbbc3..254dd25 100644 --- a/server_manager/tests/contract_locking.rs +++ b/server_manager/tests/contract_locking.rs @@ -39,3 +39,8 @@ fn test_process_lock_acquisition_and_mutual_exclusion() { let _ = fs::remove_dir_all(&temp_dir); } + +#[test] +fn test_process_lock_rejects_unsafe_path() { + assert!(ProcessLock::acquire("../unsafe.lock", true).is_err()); +}