diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..7aa0e04 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,5 @@ +## 2024-03-26 - Centralize and secure bitcoin-cli execution + +**Vulnerability:** `run_bitcoin_cli` is defined in two places (`src/utils.rs` and `src/wallet_service.rs`) and executes the `bitcoin_cli` command defined in a user-provided profile without strict validation, potentially allowing arbitrary command execution if the profile is tampered with or maliciously constructed. +**Learning:** External command execution based on configurable profile values must be strictly validated against an allowlist (e.g., only "bitcoin-cli" or "bitcoin-cli.exe") to prevent arbitrary command injection. Duplicated security-critical functions make auditing harder. +**Prevention:** Centralize external process execution and enforce strict validation on executable names derived from configurations. diff --git a/src/utils.rs b/src/utils.rs index 53162a4..36b8e60 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -104,6 +104,20 @@ pub fn run_bitcoin_cli( profile: &Profile, args: &[String], ) -> Result { + // SECURITY 🛡️: Validate binary name to prevent arbitrary command execution via configuration. + // The profile is untrusted input. We must ensure the command is exactly the expected binary. + let bin_name = std::path::Path::new(&profile.bitcoin_cli) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(""); + + if bin_name != "bitcoin-cli" && bin_name != "bitcoin-cli.exe" { + return Err(crate::error::AppError::Config(format!( + "security violation: execution of arbitrary binaries is blocked. Expected 'bitcoin-cli', got '{}'", + profile.bitcoin_cli + ))); + } + let mut cmd = std::process::Command::new(&profile.bitcoin_cli); for arg in &profile.bitcoin_cli_args { cmd.arg(arg); diff --git a/src/wallet_service.rs b/src/wallet_service.rs index c41aa76..4da7973 100644 --- a/src/wallet_service.rs +++ b/src/wallet_service.rs @@ -190,26 +190,5 @@ pub fn persist_wallet_session(session: &mut WalletSession) -> Result<(), AppErro write_profile(&session.profile_path, &session.profile) } -#[allow(dead_code)] -pub fn run_bitcoin_cli(profile: &Profile, args: &[String]) -> Result { - let output = ShellCommand::new(&profile.bitcoin_cli) - .args(&profile.bitcoin_cli_args) - .args(args) - .output() - .map_err(|e| AppError::Config(format!("failed to launch {}: {e}", profile.bitcoin_cli)))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); - let details = if !stderr.is_empty() { stderr } else { stdout }; - return Err(AppError::Network(format!( - "bitcoin-cli command failed: {} {}", - profile.bitcoin_cli, details - ))); - } - - Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) -} - #[cfg(test)] mod tests {}