From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id DDE721FF199 for ; Fri, 21 Feb 2025 11:18:39 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A92FB2C3D7; Fri, 21 Feb 2025 10:23:17 +0100 (CET) From: Maximiliano Sandoval To: pbs-devel@lists.proxmox.com Date: Fri, 21 Feb 2025 10:22:44 +0100 Message-Id: <20250221092244.128601-1-m.sandoval@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.103 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [perl.org, proxmox-daily-update.rs, proxmox-backup-proxy.rs, mod.rs, proxmox-backup-api.rs, proxmox-backup-manager.rs] Subject: [pbs-devel] [PATCH backup] mark setup_safe_path_env as unsafe X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" This method is unsafe in the 2024 edition. The specific wording used in the safety docstring comes from `cargo fix --edition`. Signed-off-by: Maximiliano Sandoval --- I would personally prefer if someone more familiarized with these 4 binaries can clarify whether all these uses are safe and add a, for example, ``` SAFETY: The enviroment is only ever accessed from here. ``` comment on top of the unsafe incantations. src/bin/proxmox-backup-api.rs | 2 +- src/bin/proxmox-backup-manager.rs | 2 +- src/bin/proxmox-backup-proxy.rs | 2 +- src/bin/proxmox-daily-update.rs | 2 +- src/tools/mod.rs | 16 ++++++++++++---- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs index 829974d25..ed1d12772 100644 --- a/src/bin/proxmox-backup-api.rs +++ b/src/bin/proxmox-backup-api.rs @@ -20,7 +20,7 @@ use proxmox_backup::server::auth::check_pbs_auth; fn main() { pbs_tools::setup_libc_malloc_opts(); - proxmox_backup::tools::setup_safe_path_env(); + unsafe { proxmox_backup::tools::setup_path_env() }; if let Err(err) = proxmox_async::runtime::main(run()) { eprintln!("Error: {}", err); diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs index 02ca0d028..910e9623d 100644 --- a/src/bin/proxmox-backup-manager.rs +++ b/src/bin/proxmox-backup-manager.rs @@ -704,7 +704,7 @@ async fn run() -> Result<(), Error> { } fn main() -> Result<(), Error> { - proxmox_backup::tools::setup_safe_path_env(); + unsafe { proxmox_backup::tools::setup_path_env() }; proxmox_async::runtime::main(run()) } diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs index 2b6ceb6e6..a2bc3d3f1 100644 --- a/src/bin/proxmox-backup-proxy.rs +++ b/src/bin/proxmox-backup-proxy.rs @@ -57,7 +57,7 @@ use proxmox_backup::server::do_verification_job; fn main() -> Result<(), Error> { pbs_tools::setup_libc_malloc_opts(); - proxmox_backup::tools::setup_safe_path_env(); + unsafe { proxmox_backup::tools::setup_path_env() }; let backup_uid = pbs_config::backup_user()?.uid; let backup_gid = pbs_config::backup_group()?.gid; diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs index dbfee07b4..7861c9a79 100644 --- a/src/bin/proxmox-daily-update.rs +++ b/src/bin/proxmox-daily-update.rs @@ -108,7 +108,7 @@ async fn run(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> { } fn main() { - proxmox_backup::tools::setup_safe_path_env(); + unsafe { proxmox_backup::tools::setup_path_env() }; if let Err(err) = syslog::init( syslog::Facility::LOG_DAEMON, diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 322894dd7..220ec445e 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -54,10 +54,18 @@ pub fn pbs_simple_http(proxy_config: Option) -> Client { Client::with_options(options) } -pub fn setup_safe_path_env() { - std::env::set_var("PATH", "/sbin:/bin:/usr/sbin:/usr/bin"); - // Make %ENV safer - as suggested by https://perldoc.perl.org/perlsec.html +/// Setups the enviroment in a safer way. +/// +/// Here "safer" should be understood as described in +/// https://perldoc.perl.org/perlsec.html. +/// +/// ## Safety +/// +/// The caller must ensure that enviroment access only happens in +/// single-threaded code. +pub unsafe fn setup_path_env() { + unsafe { std::env::set_var("PATH", "/sbin:/bin:/usr/sbin:/usr/bin") }; for name in &["IFS", "CDPATH", "ENV", "BASH_ENV"] { - std::env::remove_var(name); + unsafe { std::env::remove_var(name) }; } } -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel