🛡️ Sentinel: [CRITICAL] Fix Path Traversal vulnerability

.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-16 - Prevent Path Traversal in Profile and Snapshot Processing
+**Vulnerability:** The application was using unvalidated user input (`config.profile` and `name` in snapshot commands) to construct file paths for reading, writing, and creating profile and snapshot files (e.g. `snapshots/{name}.json`), resulting in a critical Path Traversal vulnerability.
+**Learning:** File paths concatenated using standard path joining methods (e.g. `Path::join` and `format!`) inherently trust the supplied inputs. Without explicit validation of the filename characters, relative path components like `../` will resolve and potentially bypass intended directory sandbox constraints, exposing critical state, leading to data exposure or arbitrary file overwrites.
+**Prevention:** Implement and enforce a strict path validation mechanism (e.g., `validate_file_name`) that asserts the input exclusively contains safe characters (like alphanumeric, underscores, and dashes) and rejects attempts using directories separators or empty inputs before passing the input to any file system operations.

src/commands/snapshot.rs

@@ -12,6 +12,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result<CommandOutput, AppErr
 
     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 +28,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result<CommandOutput, AppErr
             })
         }
         SnapshotAction::Restore { name } => {
+            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()));
             }

src/paths.rs

@@ -56,6 +56,7 @@ pub fn home_dir() -> PathBuf {
 }
 
 pub fn profile_path(config: &crate::config::ServiceConfig<'_>) -> Result<PathBuf, AppError> {
+    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<Pa
 }
 
 pub fn snapshot_dir(config: &crate::config::ServiceConfig<'_>) -> Result<PathBuf, AppError> {
+    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)

src/utils.rs

@@ -185,6 +185,22 @@ pub fn resolve_psbt_source(
     ))
 }
 
+pub fn validate_file_name(name: &str) -> Result<(), AppError> {
+    if name.is_empty() {
+        return Err(AppError::Invalid("filename cannot be empty".to_string()));
+    }
+    if !name
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+    {
+        return Err(AppError::Invalid(
+            "filename can only contain alphanumeric characters, underscores, and dashes"
+                .to_string(),
+        ));
+    }
+    Ok(())
+}
+
 pub fn parse_indices(s: Option<&str>) -> Result<Vec<usize>, AppError> {
     let s = match s {
         Some(s) => s,