From a8b3e147fe46e1f664db14c636750e4777b9721e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 24 Aug 2022 09:17:12 -0400 Subject: [PATCH] arti_client: Refuse to build a client if we are setuid. Arti is not designed to be a setuid-safe program. Part of #523. --- Cargo.lock | 1 + crates/arti-client/Cargo.toml | 1 + crates/arti-client/src/client.rs | 7 +++++++ crates/arti-client/src/util.rs | 36 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 37848aaaa..8df231f42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -151,6 +151,7 @@ dependencies = [ "fs-mistrust", "futures", "humantime-serde", + "libc", "once_cell", "pin-project", "postage", diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml index f188e2e49..edd72d901 100644 --- a/crates/arti-client/Cargo.toml +++ b/crates/arti-client/Cargo.toml @@ -60,6 +60,7 @@ educe = "0.4.6" fs-mistrust = { path = "../fs-mistrust", version = "0.4.0", features = ["serde"] } futures = "0.3.14" humantime-serde = "1.1.1" +libc = "0.2" pin-project = "1" postage = { version = "0.5.0", default-features = false, features = ["futures-traits"] } safelog = { path = "../safelog", version = "0.1.0" } diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index 2c2560b45..df8dcbb5d 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -369,6 +369,13 @@ impl TorClient { dirmgr_builder: &dyn crate::builder::DirProviderBuilder, dirmgr_extensions: tor_dirmgr::config::DirMgrExtensions, ) -> StdResult { + if crate::util::running_as_setuid() { + return Err(tor_error::bad_api_usage!( + "Arti does not support running in a setuid or setgid context." + ) + .into()); + } + let dormant = DormantMode::Normal; let dir_cfg = { let mut c: tor_dirmgr::DirMgrConfig = config.dir_mgr_config()?; diff --git a/crates/arti-client/src/util.rs b/crates/arti-client/src/util.rs index 12db4531e..c6d29647e 100644 --- a/crates/arti-client/src/util.rs +++ b/crates/arti-client/src/util.rs @@ -27,3 +27,39 @@ impl<'a, T: StateMgr + 'a> StateMgrUnlockGuard<'a, T> { std::mem::forget(self); } } + +/// Return true if we are running with elevated privileges via setuid, setgid, +/// or a similar mechanism. +/// +/// We detect this by checking whether there is any difference between our real, +/// effective, and saved user IDs; and then by doing the same check for our +/// group IDs. +/// +/// On non-unix platforms, this function always returns false. +pub(crate) fn running_as_setuid() -> bool { + #[cfg(target_family = "unix")] + { + // Use `libc` to find our real, effective, and saved UIDs and GIDs. + let mut resuid = [0, 0, 0]; + let mut resgid = [0, 0, 0]; + unsafe { + // We ignore failures from getresuid or getresgid: these syscalls + // can only fail if we give them bad pointers, if they are disabled + // via a maddened sandbox, or something like that. In that case, we'll + // just assume that the user knows what they're doing. + let _ = libc::getresuid(&mut resuid[0], &mut resuid[1], &mut resuid[2]); + let _ = libc::getresgid(&mut resgid[0], &mut resgid[1], &mut resgid[2]); + } + + // The user can change any of their (real, effective, saved) IDs to any + // of their (real, effective, saved) IDs. Thus, privileges are elevated + // if there is any difference between these IDs. + let same_resuid = resuid.iter().all(|uid| uid == &resuid[0]); + let same_resgid = resgid.iter().all(|gid| gid == &resgid[0]); + !(same_resuid && same_resgid) + } + #[cfg(not(target_family = "unix"))] + { + false + } +}