[pbs-devel] [PATCH backup] mark setup_safe_path_env as unsafe
Maximiliano Sandoval
m.sandoval at proxmox.com
Fri Feb 21 10:22:44 CET 2025
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 <m.sandoval at proxmox.com>
---
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<ProxyConfig>) -> 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
More information about the pbs-devel
mailing list