From 66769dea0f43066e6fa443a552cf3654efa858cd Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 26 Mar 2026 12:45:30 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20arbitrary=20command=20execution=20via=20profile.bit?= =?UTF-8?q?coin=5Fcli?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: CRITICAL 💡 Vulnerability: `run_bitcoin_cli` executed the `bitcoin_cli` command defined in a user-provided profile without strict validation, potentially allowing arbitrary command execution if the profile was tampered with or maliciously constructed. Also removed duplicate implementation from `src/wallet_service.rs` which increased attack surface. 🎯 Impact: A malicious profile could execute arbitrary binaries on the host system with the arguments intended for bitcoin-cli. 🔧 Fix: Centralized the command execution in `src/utils.rs` and added an allowlist check on the binary filename, restricting execution to only `bitcoin-cli` or `bitcoin-cli.exe`. ✅ Verification: Verify by configuring a profile with a `bitcoin_cli` value other than `bitcoin-cli` or `bitcoin-cli.exe` and attempting any command that uses `bitcoin-cli`. It should fail with a "security violation" error. Co-authored-by: bitcoiner-dev <75873427+bitcoiner-dev@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ src/utils.rs | 14 ++++++++++++++ src/wallet_service.rs | 21 --------------------- 3 files changed, 19 insertions(+), 21 deletions(-) create mode 100644 .jules/sentinel.md 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 {} From 33f2c6c0fb2db86503ba8a41d19575a69e6398f6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 26 Mar 2026 20:00:42 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20arbitrary=20command=20execution=20via=20profile.bit?= =?UTF-8?q?coin=5Fcli?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: CRITICAL 💡 Vulnerability: `run_bitcoin_cli` executed the `bitcoin_cli` command defined in a user-provided profile without strict validation, potentially allowing arbitrary command execution if the profile was tampered with or maliciously constructed. Also removed duplicate implementation from `src/wallet_service.rs` which increased attack surface. 🎯 Impact: A malicious profile could execute arbitrary binaries on the host system with the arguments intended for bitcoin-cli. 🔧 Fix: Centralized the command execution in `src/utils.rs` and added an allowlist check on the binary filename, restricting execution to only `bitcoin-cli` or `bitcoin-cli.exe`. ✅ Verification: Verify by configuring a profile with a `bitcoin_cli` value other than `bitcoin-cli` or `bitcoin-cli.exe` and attempting any command that uses `bitcoin-cli`. It should fail with a "security violation" error. Co-authored-by: bitcoiner-dev <75873427+bitcoiner-dev@users.noreply.github.com>