🛡️ Sentinel: [CRITICAL] Fix path traversal in snapshot#41
Conversation
Implemented strict filename validation in `src/utils.rs` (`validate_file_name`) and applied it to the snapshot `Save` and `Restore` commands. This prevents a path traversal vulnerability where maliciously formatted profile names (e.g., containing `../`) could be passed to `Path::join`, enabling the read or overwrite of arbitrary files outside the snapshot directory. Tests added to confirm validation behavior. Co-authored-by: bitcoiner-dev <75873427+bitcoiner-dev@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: The
snapshotcommand'sSaveandRestoreactions accepted arbitrary user input (name) and passed it directly to Rust'sPath::join()method (e.g.,snap_dir.join(format!("{name}.json"))). If the input contained characters like../or was an absolute path, it could traverse outside the intended base directory.🎯 Impact: A user or automated script manipulating the
nameargument could potentially read arbitrary configuration files by trickingsnapshot restore, or overwrite sensitive local files (like~/.ssh/id_rsaor system configuration files) viasnapshot save. This could lead to a severe privilege escalation or information leakage.🔧 Fix: Introduced a secure allowlist function
crate::utils::validate_file_namethat restricts valid names to only ASCII alphanumeric characters, dashes (-), and underscores (_). Applied this validation synchronously inside the snapshot command prior to evaluating paths.✅ Verification: Run
cargo test --bin zinc-clito verify new unit tests, and manually attempt to runcargo run -- snapshot save "../../etc/passwd"to confirm it is explicitly rejected with an 'invalid characters' error.PR created automatically by Jules for task 15782383214298169592 started by @bitcoiner-dev