diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 986d7ff..1ec6a40 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-17 - Path Traversal in Snapshot Commands +**Vulnerability:** The snapshot command allowed user-supplied snapshot names (via `args.name`) to be directly formatted into a path and joined with the snapshot directory (`snap_dir.join(format!("{name}.json"))`). This lack of validation creates a path traversal vulnerability where a user could provide a name like `../../../etc/passwd` (or similar) to read from or write to arbitrary locations on the file system, bypassing intended directory restrictions. +**Learning:** Using user input directly in file paths is extremely dangerous, even if the application appends an extension like `.json`, as an attacker can still control the destination directory. +**Prevention:** Always validate user-provided file names using strict allowlists (e.g., alphanumeric, underscores, dashes) via utilities like `crate::utils::validate_file_name` before using them in file path construction. diff --git a/src/commands/snapshot.rs b/src/commands/snapshot.rs index 7be1d9e..d4b1143 100644 --- a/src/commands/snapshot.rs +++ b/src/commands/snapshot.rs @@ -12,6 +12,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result { + 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 { + 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/utils.rs b/src/utils.rs index 8e3996f..d78166f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -91,6 +91,20 @@ pub(crate) fn best_match<'a>(needle: &str, candidates: &'a [&'a str]) -> Option< } } +pub fn validate_file_name(name: &str) -> Result<(), AppError> { + if name.is_empty() + || !name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + return Err(AppError::Invalid(format!( + "invalid file name: '{}' (only alphanumeric, underscores, and dashes are allowed)", + name + ))); + } + Ok(()) +} + pub fn maybe_write_text(path: Option<&str>, text: &str) -> Result<(), crate::error::AppError> { if let Some(path) = path { crate::paths::write_secure_file(path, text.as_bytes()) diff --git a/src/utils.rs.orig b/src/utils.rs.orig new file mode 100644 index 0000000..8e3996f --- /dev/null +++ b/src/utils.rs.orig @@ -0,0 +1,221 @@ +use crate::config::{NetworkArg, Profile, SchemeArg}; +use crate::error::AppError; +use std::env; +use std::path::{Path, PathBuf}; + +pub fn home_dir() -> PathBuf { + if let Some(home) = std::env::var_os("HOME") { + PathBuf::from(home) + } else { + PathBuf::from(".") + } +} + +pub fn env_non_empty(name: &str) -> Option { + let value = env::var(name).ok()?; + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } +} + +pub fn env_bool(name: &str) -> Option { + let value = env_non_empty(name)?; + let normalized = value.to_ascii_lowercase(); + match normalized.as_str() { + "1" | "true" | "yes" | "on" => Some(true), + "0" | "false" | "no" | "off" => Some(false), + _ => None, + } +} + +pub fn parse_network(s: &str) -> Result { + match s.to_lowercase().as_str() { + "bitcoin" | "mainnet" => Ok(NetworkArg::Bitcoin), + "signet" => Ok(NetworkArg::Signet), + "testnet" => Ok(NetworkArg::Testnet), + "regtest" => Ok(NetworkArg::Regtest), + _ => Err(AppError::Invalid(format!("unknown network: {s}"))), + } +} + +pub fn parse_scheme(s: &str) -> Result { + match s.to_lowercase().as_str() { + "unified" => Ok(SchemeArg::Unified), + "dual" => Ok(SchemeArg::Dual), + _ => Err(AppError::Invalid(format!("unknown scheme: {s}"))), + } +} + +pub(crate) fn parse_bool_value(value: &str, context: &str) -> Result { + let normalized = value.trim().to_ascii_lowercase(); + match normalized.as_str() { + "1" | "true" | "yes" | "on" => Ok(true), + "0" | "false" | "no" | "off" => Ok(false), + _ => Err(format!( + "{context} must be one of: true,false,yes,no,on,off,1,0" + )), + } +} + +pub(crate) fn unknown_with_hint(kind: &str, unknown: &str, candidates: &[&str]) -> String { + if let Some(suggestion) = best_match(unknown, candidates) { + return format!("unknown {kind}: {unknown} (did you mean {suggestion}?)"); + } + format!("unknown {kind}: {unknown}") +} + +pub(crate) fn best_match<'a>(needle: &str, candidates: &'a [&'a str]) -> Option<&'a str> { + let mut best: Option<(&str, usize)> = None; + for &candidate in candidates { + let score = levenshtein(needle, candidate); + match best { + Some((_, best_score)) if score >= best_score => {} + _ => best = Some((candidate, score)), + } + } + + let (candidate, score) = best?; + let threshold = match needle.len() { + 0..=4 => 1, + 5..=9 => 2, + _ => 3, + }; + + if score <= threshold { + Some(candidate) + } else { + None + } +} + +pub fn maybe_write_text(path: Option<&str>, text: &str) -> Result<(), crate::error::AppError> { + if let Some(path) = path { + crate::paths::write_secure_file(path, text.as_bytes()) + .map_err(|e| crate::error::AppError::Io(format!("failed to write to {path}: {e}"))) + } else { + Ok(()) + } +} + +pub fn run_bitcoin_cli( + profile: &Profile, + args: &[String], +) -> Result { + let mut cmd = std::process::Command::new(&profile.bitcoin_cli); + for arg in &profile.bitcoin_cli_args { + cmd.arg(arg); + } + cmd.args(args); + let output = cmd + .output() + .map_err(|e| crate::error::AppError::Internal(format!("bitcoin-cli failed: {e}")))?; + if !output.status.success() { + let err = String::from_utf8_lossy(&output.stderr); + return Err(crate::error::AppError::Internal(format!( + "bitcoin-cli error: {err}" + ))); + } + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) +} + +pub(crate) fn levenshtein(a: &str, b: &str) -> usize { + if a == b { + return 0; + } + if a.is_empty() { + return b.chars().count(); + } + if b.is_empty() { + return a.chars().count(); + } + + let b_len = b.chars().count(); + let mut prev: Vec = (0..=b_len).collect(); + let mut curr = vec![0; b_len + 1]; + + for (i, ca) in a.chars().enumerate() { + curr[0] = i + 1; + for (j, cb) in b.chars().enumerate() { + let cost = usize::from(ca != cb); + curr[j + 1] = (curr[j] + 1).min(prev[j + 1] + 1).min(prev[j] + cost); + } + std::mem::swap(&mut prev, &mut curr); + } + + prev[b_len] +} +pub fn resolve_psbt_source( + psbt: Option<&str>, + psbt_file: Option<&Path>, + psbt_stdin: bool, +) -> Result { + let count = (psbt.is_some() as u8) + (psbt_file.is_some() as u8) + (psbt_stdin as u8); + if count > 1 { + return Err(AppError::Invalid( + "accepts only one of --psbt, --psbt-file, --psbt-stdin".to_string(), + )); + } + if let Some(psbt) = psbt { + return Ok(psbt.to_string()); + } + if let Some(path) = psbt_file { + return std::fs::read_to_string(path).map_err(|e| { + AppError::Io(format!("failed to read psbt file {}: {e}", path.display())) + }); + } + if psbt_stdin { + use std::io::Read; + let mut buffer = String::new(); + std::io::stdin() + .read_to_string(&mut buffer) + .map_err(|e| AppError::Io(format!("failed to read psbt from stdin: {e}")))?; + let trimmed = buffer.trim(); + if trimmed.is_empty() { + return Err(AppError::Invalid( + "stdin did not contain a PSBT string".to_string(), + )); + } + return Ok(trimmed.to_string()); + } + Err(AppError::Invalid( + "requires one of --psbt, --psbt-file, --psbt-stdin".to_string(), + )) +} + +pub fn parse_indices(s: Option<&str>) -> Result, AppError> { + let s = match s { + Some(s) => s, + None => return Ok(Vec::new()), + }; + let mut indices = Vec::new(); + for part in s.split(',') { + let part = part.trim(); + if part.is_empty() { + continue; + } + if part.contains('-') { + let bounds: Vec<&str> = part.split('-').collect(); + if bounds.len() != 2 { + return Err(AppError::Invalid(format!("invalid index range: {part}"))); + } + let start: usize = bounds[0] + .parse() + .map_err(|_| AppError::Invalid(format!("invalid start index: {}", bounds[0])))?; + let end: usize = bounds[1] + .parse() + .map_err(|_| AppError::Invalid(format!("invalid end index: {}", bounds[1])))?; + for i in start..=end { + indices.push(i); + } + } else { + let index: usize = part + .parse() + .map_err(|_| AppError::Invalid(format!("invalid index: {part}")))?; + indices.push(index); + } + } + Ok(indices) +}