From c4bb390f51cc54136ffe231af3d6214bb089cb88 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 13 Apr 2026 13:41:45 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20path=20traversal=20in=20profile=20and=20snapshot=20pat?= =?UTF-8?q?hs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: bitcoiner-dev <75873427+bitcoiner-dev@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ src/commands/snapshot.rs | 4 ++-- src/paths.rs | 2 ++ src/utils.rs | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 986d7ff..051e2ff 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -7,3 +7,8 @@ **Vulnerability:** The `maybe_write_text` utility function was using `std::fs::write`, which resulted in sensitive data (like PSBT files and offers) being saved with insecure default file permissions, making them readable by other users on a shared system. **Learning:** Even generic utility functions used for saving user-requested command outputs must use secure file permissions (`0o600`) if the data they handle (like PSBTs and offers) is sensitive. **Prevention:** Always use `crate::paths::write_secure_file` instead of `std::fs::write` for all file writing operations that might contain sensitive material in this codebase. + +## 2024-04-13 - Path Traversal on Profile and Snapshot Names +**Vulnerability:** The application was constructing file paths for profiles (`profiles/{profile_name}.json`) and snapshots (`snapshots/{profile_name}/{snapshot_name}.json`) without sanitizing or validating the `profile_name` or `snapshot_name` variables. This could allow an attacker to read, write, or overwrite arbitrary files on the system using path traversal payloads like `../../`. +**Learning:** Any user-provided or configurable string that is used to construct a file path must be strictly validated to prevent path traversal, even if the application otherwise handles the actual file read/write operations securely. +**Prevention:** Use a dedicated validation function (e.g., `crate::utils::validate_file_name`) to enforce a strict allowlist of characters (such as alphanumeric, underscores, and dashes) on file and directory names before using them in `Path::join` operations. diff --git a/src/commands/snapshot.rs b/src/commands/snapshot.rs index 7be1d9e..944b696 100644 --- a/src/commands/snapshot.rs +++ b/src/commands/snapshot.rs @@ -7,11 +7,10 @@ use std::fs; pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result { let profile_path = profile_path(cli)?; let snap_dir = snapshot_dir(cli)?; - fs::create_dir_all(&snap_dir) - .map_err(|e| AppError::Config(format!("failed to create snapshot dir: {e}")))?; match &args.action { SnapshotAction::Save { name, overwrite } => { + crate::utils::validate_file_name(name)?; let source = read_profile(&profile_path)?; let destination = snap_dir.join(format!("{name}.json")); if destination.exists() && !(*overwrite || cli.yes) { @@ -27,6 +26,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result { + crate::utils::validate_file_name(name)?; if !confirm(&format!("Are you sure you want to restore snapshot '{name}'? This will overwrite your current profile."), cli) { return Err(AppError::Internal("aborted by user".to_string())); } diff --git a/src/paths.rs b/src/paths.rs index e9be32c..8fa55e4 100644 --- a/src/paths.rs +++ b/src/paths.rs @@ -56,6 +56,7 @@ pub fn home_dir() -> PathBuf { } pub fn profile_path(config: &crate::config::ServiceConfig<'_>) -> Result { + crate::utils::validate_file_name(config.profile)?; let root = data_dir(config); let profiles = root.join("profiles"); if !profiles.exists() { @@ -70,6 +71,7 @@ pub fn profile_lock_path(config: &crate::config::ServiceConfig<'_>) -> Result) -> Result { + crate::utils::validate_file_name(config.profile)?; let root = data_dir(config); let directory = root.join("snapshots").join(config.profile); create_secure_dir_all(&directory) diff --git a/src/utils.rs b/src/utils.rs index 8e3996f..4f3af5c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -219,3 +219,19 @@ pub fn parse_indices(s: Option<&str>) -> Result, AppError> { } Ok(indices) } + +pub fn validate_file_name(name: &str) -> Result<(), AppError> { + if name.is_empty() { + return Err(AppError::Invalid("file name cannot be empty".to_string())); + } + if !name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + return Err(AppError::Invalid( + "file name can only contain alphanumeric characters, underscores, and dashes" + .to_string(), + )); + } + Ok(()) +}