Repository: GuillaumeGomez/sysinfo (rust)

Issue:
Wrong userid for process sometimes returned on Linux
**Describe the bug**

sysinfo version 0.28.3, platform is Amazon Linux 2 (also reproed on Ubuntu)

`sysinfo` always reports a userid of `root` on Linux for processes with the `dumpable` attribute set to a value other than `1`. This is because it attempts to get the UID by `stat`-ing `/proc/[pid]/status` [here](https://github.com/GuillaumeGomez/sysinfo/blob/master/src/linux/process.rs#L654) and picking the owner of that file. The problem with this is that `/proc/[pid]/status` isn't always owned by the process's owner. From the [docs](https://man7.org/linux/man-pages/man5/proc.5.html)  for `proc`:

> The files inside each `/proc/[pid]` directory are normally
              owned by the effective user and effective group ID of the
              process.  However, as a security measure, the ownership is
              made root:root if the process's "dumpable" attribute is
              set to a value other than 1.

See [here](https://man7.org/linux/man-pages/man2/prctl.2.html) for docs on a few scenarios in which this flag can get unset.

Note that the `/proc/[pid]` directory itself is always owned by the process owner, so `stat`-ing that instead would be a good alternative.

**To Reproduce**

I put together a small repository with a repro https://github.com/nsunderland1/sysinfo_bug

The meat of it is in `main.rs` https://github.com/nsunderland1/sysinfo_bug/blob/main/src/main.rs

```rust
use std::ops::Deref;

use sysinfo::{ProcessExt, ProcessRefreshKind, RefreshKind, System, SystemExt};

fn main() {
    // The user passes a UID from the command line
    let my_new_uid: u32 = std::env::args().nth(1).unwrap().parse().unwrap();
    // It shouldn't match the current process's effective UID
    assert_ne!(nix::unistd::Uid::effective().as_raw(), my_new_uid);
    // Now use seteuid to change our effective UID to it
    nix::unistd::seteuid(nix::unistd::Uid::from_raw(my_new_uid)).unwrap();
    // Sanity check that it worked
    assert_eq!(nix::unistd::Uid::effective().as_raw(), my_new_uid);

    // Now check the effective UID using sysinfo
    let system = System::new_with_specifics(
        RefreshKind::new().with_processes(ProcessRefreshKind::new().with_user()),
    );

    let my_pid = sysinfo::get_current_pid().unwrap();
    let my_uid_but_wrong = system.process(my_pid).unwrap().user_id().unwrap();

    // This assertion will fail, as sysinfo will report a UID of 0 (root)
    assert_eq!(*my_uid_but_wrong.deref(), my_new_uid);
}
```

Pull request:
Add Process::{effective_user_id, effective_group_id} + user_id & group_id to return real IDs
Add API to query effective IDs of a process and make `Process::{user_id, group_id}` return real IDs on Linux and Apple, so the behavior is as expected and consistent with the FreeBSD implementation.

Fixes #970.

---

Zeeshan Ali Khan on behalf of MBition GmbH.

[Provider Information](https://github.com/mercedes-benz/foss/blob/master/PROVIDER_INFORMATION.md#mbition-gmbh).

Patch:
diff --git a/src/apple/app_store/process.rs b/src/apple/app_store/process.rs
index b5de095..d7df72a 100644
--- a/src/apple/app_store/process.rs
+++ b/src/apple/app_store/process.rs
@@ -76,10 +76,18 @@ impl ProcessExt for Process {
         None
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        None
+    }
+
     fn group_id(&self) -> Option<Gid> {
         None
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        None
+    }
+
     fn wait(&self) {}
 
     fn session_id(&self) -> Option<Pid> {
diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs
index 489e124..ad546f2 100644
--- a/src/apple/macos/process.rs
+++ b/src/apple/macos/process.rs
@@ -33,13 +33,15 @@ pub struct Process {
     pub(crate) updated: bool,
     cpu_usage: f32,
     user_id: Option<Uid>,
+    effective_user_id: Option<Uid>,
     group_id: Option<Gid>,
+    effective_group_id: Option<Gid>,
     pub(crate) process_status: ProcessStatus,
     /// Status of process (running, stopped, waiting, etc). `None` means `sysinfo` doesn't have
     /// enough rights to get this information.
     ///
     /// This is very likely this one that you want instead of `process_status`.
-    pub(crate) status: Option<ThreadStatus>,
+    pub status: Option<ThreadStatus>,
     pub(crate) old_read_bytes: u64,
     pub(crate) old_written_bytes: u64,
     pub(crate) read_bytes: u64,
@@ -66,7 +68,9 @@ impl Process {
             start_time: 0,
             run_time: 0,
             user_id: None,
+            effective_user_id: None,
             group_id: None,
+            effective_group_id: None,
             process_status: ProcessStatus::Unknown(0),
             status: None,
             old_read_bytes: 0,
@@ -95,7 +99,9 @@ impl Process {
             start_time,
             run_time,
             user_id: None,
+            effective_user_id: None,
             group_id: None,
+            effective_group_id: None,
             process_status: ProcessStatus::Unknown(0),
             status: None,
             old_read_bytes: 0,
@@ -153,13 +159,6 @@ impl ProcessExt for Process {
     }
 
     fn status(&self) -> ProcessStatus {
-        // If the status is `Run`, then it's very likely wrong so we instead
-        // return a `ProcessStatus` converted from the `ThreadStatus`.
-        if self.process_status == ProcessStatus::Run {
-            if let Some(thread_status) = self.status {
-                return ProcessStatus::from(thread_status);
-            }
-        }
         self.process_status
     }
 
@@ -188,10 +187,18 @@ impl ProcessExt for Process {
         self.user_id.as_ref()
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        self.effective_user_id.as_ref()
+    }
+
     fn group_id(&self) -> Option<Gid> {
         self.group_id
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        self.effective_group_id
+    }
+
     fn wait(&self) {
         let mut status = 0;
         // attempt waiting
@@ -533,8 +540,10 @@ unsafe fn create_new_process(
     p.memory = task_info.pti_resident_size;
     p.virtual_memory = task_info.pti_virtual_size;
 
-    p.user_id = Some(Uid(info.pbi_uid));
-    p.group_id = Some(Gid(info.pbi_gid));
+    p.user_id = Some(Uid(info.pbi_ruid));
+    p.effective_user_id = Some(Uid(info.pbi_uid));
+    p.group_id = Some(Gid(info.pbi_rgid));
+    p.effective_group_id = Some(Gid(info.pbi_gid));
     p.process_status = ProcessStatus::from(info.pbi_status);
     if refresh_kind.disk_usage() {
         update_proc_disk_activity(&mut p);
diff --git a/src/apple/process.rs b/src/apple/process.rs
index f44ae6f..e0f005b 100644
--- a/src/apple/process.rs
+++ b/src/apple/process.rs
@@ -5,41 +5,20 @@ use std::fmt;
 pub use crate::sys::inner::process::*;
 use crate::ProcessStatus;
 
-// FIXME: To be removed once <https://github.com/rust-lang/libc/pull/3233> is merged and released.
-const SIDL: u32 = 1;
-const SRUN: u32 = 2;
-const SSLEEP: u32 = 3;
-const SSTOP: u32 = 4;
-const SZOMB: u32 = 5;
-
 #[doc(hidden)]
 impl From<u32> for ProcessStatus {
     fn from(status: u32) -> ProcessStatus {
         match status {
-            SIDL => ProcessStatus::Idle,
-            SRUN => ProcessStatus::Run,
-            SSLEEP => ProcessStatus::Sleep,
-            SSTOP => ProcessStatus::Stop,
-            SZOMB => ProcessStatus::Zombie,
+            1 => ProcessStatus::Idle,
+            2 => ProcessStatus::Run,
+            3 => ProcessStatus::Sleep,
+            4 => ProcessStatus::Stop,
+            5 => ProcessStatus::Zombie,
             x => ProcessStatus::Unknown(x),
         }
     }
 }
 
-#[doc(hidden)]
-impl From<ThreadStatus> for ProcessStatus {
-    fn from(status: ThreadStatus) -> ProcessStatus {
-        match status {
-            ThreadStatus::Running => ProcessStatus::Run,
-            ThreadStatus::Stopped => ProcessStatus::Stop,
-            ThreadStatus::Waiting => ProcessStatus::Sleep,
-            ThreadStatus::Uninterruptible => ProcessStatus::Dead,
-            ThreadStatus::Halted => ProcessStatus::Parked,
-            ThreadStatus::Unknown(x) => ProcessStatus::Unknown(x as _),
-        }
-    }
-}
-
 impl fmt::Display for ProcessStatus {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.write_str(match *self {
@@ -54,8 +33,8 @@ impl fmt::Display for ProcessStatus {
 }
 
 /// Enum describing the different status of a thread.
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub(crate) enum ThreadStatus {
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum ThreadStatus {
     /// Thread is running normally.
     Running,
     /// Thread is stopped.
@@ -73,12 +52,32 @@ pub(crate) enum ThreadStatus {
 impl From<i32> for ThreadStatus {
     fn from(status: i32) -> ThreadStatus {
         match status {
-            libc::TH_STATE_RUNNING => ThreadStatus::Running,
-            libc::TH_STATE_STOPPED => ThreadStatus::Stopped,
-            libc::TH_STATE_WAITING => ThreadStatus::Waiting,
-            libc::TH_STATE_UNINTERRUPTIBLE => ThreadStatus::Uninterruptible,
-            libc::TH_STATE_HALTED => ThreadStatus::Halted,
+            1 => ThreadStatus::Running,
+            2 => ThreadStatus::Stopped,
+            3 => ThreadStatus::Waiting,
+            4 => ThreadStatus::Uninterruptible,
+            5 => ThreadStatus::Halted,
             x => ThreadStatus::Unknown(x),
         }
     }
 }
+
+impl ThreadStatus {
+    /// Used to display `ThreadStatus`.
+    pub fn to_string(&self) -> &str {
+        match *self {
+            ThreadStatus::Running => "Running",
+            ThreadStatus::Stopped => "Stopped",
+            ThreadStatus::Waiting => "Waiting",
+            ThreadStatus::Uninterruptible => "Uninterruptible",
+            ThreadStatus::Halted => "Halted",
+            ThreadStatus::Unknown(_) => "Unknown",
+        }
+    }
+}
+
+impl fmt::Display for ThreadStatus {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.to_string())
+    }
+}
diff --git a/src/common.rs b/src/common.rs
index 77de714..7a305e8 100644
--- a/src/common.rs
+++ b/src/common.rs
@@ -910,10 +910,6 @@ pub enum ProcessStatus {
     ///
     /// Parked (Linux 3.9 to 3.13 only).
     ///
-    /// ## macOS
-    ///
-    /// Halted at a clean point.
-    ///
     /// ## Other OS
     ///
     /// Not available.
diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs
index 23799c9..52064e1 100644
--- a/src/freebsd/process.rs
+++ b/src/freebsd/process.rs
@@ -58,7 +58,9 @@ pub struct Process {
     run_time: u64,
     pub(crate) status: ProcessStatus,
     user_id: Uid,
+    effective_user_id: Uid,
     group_id: Gid,
+    effective_group_id: Gid,
     read_bytes: u64,
     old_read_bytes: u64,
     written_bytes: u64,
@@ -140,10 +142,18 @@ impl ProcessExt for Process {
         Some(&self.user_id)
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        Some(&self.effective_user_id)
+    }
+
     fn group_id(&self) -> Option<Gid> {
         Some(self.group_id)
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        Some(self.effective_group_id)
+    }
+
     fn wait(&self) {
         let mut status = 0;
         // attempt waiting
@@ -259,7 +269,9 @@ pub(crate) unsafe fn get_process_data(
         pid: Pid(kproc.ki_pid),
         parent,
         user_id: Uid(kproc.ki_ruid),
+        effective_user_id: Uid(kproc.ki_uid),
         group_id: Gid(kproc.ki_rgid),
+        effective_group_id: Gid(kproc.ki_svgid),
         start_time,
         run_time: now.saturating_sub(start_time),
         cpu_usage,
diff --git a/src/linux/process.rs b/src/linux/process.rs
index 975fe3e..55893ce 100644
--- a/src/linux/process.rs
+++ b/src/linux/process.rs
@@ -5,7 +5,6 @@ use std::collections::HashMap;
 use std::fmt;
 use std::fs::{self, File};
 use std::io::Read;
-use std::mem::MaybeUninit;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
@@ -79,7 +78,9 @@ pub struct Process {
     pub(crate) updated: bool,
     cpu_usage: f32,
     user_id: Option<Uid>,
+    effective_user_id: Option<Uid>,
     group_id: Option<Gid>,
+    effective_group_id: Option<Gid>,
     pub(crate) status: ProcessStatus,
     /// Tasks run by this process.
     pub tasks: HashMap<Pid, Process>,
@@ -113,7 +114,9 @@ impl Process {
             start_time: 0,
             run_time: 0,
             user_id: None,
+            effective_user_id: None,
             group_id: None,
+            effective_group_id: None,
             status: ProcessStatus::Unknown(0),
             tasks: if pid.0 == 0 {
                 HashMap::with_capacity(1000)
@@ -204,10 +207,18 @@ impl ProcessExt for Process {
         self.user_id.as_ref()
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        self.effective_user_id.as_ref()
+    }
+
     fn group_id(&self) -> Option<Gid> {
         self.group_id
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        self.effective_group_id
+    }
+
     fn wait(&self) {
         let mut status = 0;
         // attempt waiting
@@ -337,9 +348,13 @@ fn get_status(p: &mut Process, part: &str) {
 }
 
 fn refresh_user_group_ids<P: PathPush>(p: &mut Process, path: &mut P) {
-    if let Some((user_id, group_id)) = get_uid_and_gid(path.join("status")) {
+    if let Some(((user_id, effective_user_id), (group_id, effective_group_id))) =
+        get_uid_and_gid(path.join("status"))
+    {
         p.user_id = Some(Uid(user_id));
+        p.effective_user_id = Some(Uid(effective_user_id));
         p.group_id = Some(Gid(group_id));
+        p.effective_group_id = Some(Gid(effective_group_id));
     }
 }
 
@@ -643,43 +658,38 @@ fn copy_from_file(entry: &Path) -> Vec<String> {
     }
 }
 
-fn get_uid_and_gid(file_path: &Path) -> Option<(uid_t, gid_t)> {
-    use std::os::unix::ffi::OsStrExt;
-
-    unsafe {
-        let mut sstat: MaybeUninit<libc::stat> = MaybeUninit::uninit();
-
-        let mut file_path: Vec<u8> = file_path.as_os_str().as_bytes().to_vec();
-        file_path.push(0);
-        if libc::stat(file_path.as_ptr() as *const _, sstat.as_mut_ptr()) == 0 {
-            let sstat = sstat.assume_init();
-
-            return Some((sstat.st_uid, sstat.st_gid));
-        }
-    }
-
+// Fetch tuples of real and effective UID and GID.
+fn get_uid_and_gid(file_path: &Path) -> Option<((uid_t, uid_t), (gid_t, gid_t))> {
     let status_data = get_all_data(file_path, 16_385).ok()?;
 
     // We're only interested in the lines starting with Uid: and Gid:
-    // here. From these lines, we're looking at the second entry to get
-    // the effective u/gid.
+    // here. From these lines, we're looking at the first and second entries to get
+    // the real u/gid.
 
-    let f = |h: &str, n: &str| -> Option<uid_t> {
+    let f = |h: &str, n: &str| -> (Option<uid_t>, Option<uid_t>) {
         if h.starts_with(n) {
-            h.split_whitespace().nth(2).unwrap_or("0").parse().ok()
+            let mut ids = h.split_whitespace();
+            let real = ids.nth(1).unwrap_or("0").parse().ok();
+            let effective = ids.next().unwrap_or("0").parse().ok();
+
+            (real, effective)
         } else {
-            None
+            (None, None)
         }
     };
     let mut uid = None;
+    let mut effective_uid = None;
     let mut gid = None;
+    let mut effective_gid = None;
     for line in status_data.lines() {
-        if let Some(u) = f(line, "Uid:") {
-            debug_assert!(uid.is_none());
-            uid = Some(u);
-        } else if let Some(g) = f(line, "Gid:") {
-            debug_assert!(gid.is_none());
-            gid = Some(g);
+        if let (Some(real), Some(effective)) = f(line, "Uid:") {
+            debug_assert!(uid.is_none() && effective_uid.is_none());
+            uid = Some(real);
+            effective_uid = Some(effective);
+        } else if let (Some(real), Some(effective)) = f(line, "Gid:") {
+            debug_assert!(gid.is_none() && effective_gid.is_none());
+            gid = Some(real);
+            effective_gid = Some(effective);
         } else {
             continue;
         }
@@ -687,8 +697,10 @@ fn get_uid_and_gid(file_path: &Path) -> Option<(uid_t, gid_t)> {
             break;
         }
     }
-    match (uid, gid) {
-        (Some(u), Some(g)) => Some((u, g)),
+    match (uid, effective_uid, gid, effective_gid) {
+        (Some(uid), Some(effective_uid), Some(gid), Some(effective_gid)) => {
+            Some(((uid, effective_uid), (gid, effective_gid)))
+        }
         _ => None,
     }
 }
diff --git a/src/traits.rs b/src/traits.rs
index 3bf3b02..314e61b 100644
--- a/src/traits.rs
+++ b/src/traits.rs
@@ -403,6 +403,26 @@ pub trait ProcessExt: Debug {
     /// ```
     fn user_id(&self) -> Option<&Uid>;
 
+    /// Returns the user ID of the effective owner of this process or `None` if this information
+    /// couldn't be retrieved. If you want to get the [`User`] from it, take a look at
+    /// [`SystemExt::get_user_by_id`].
+    ///
+    /// If you run something with `sudo`, the real user ID of the launched process will be the ID of
+    /// the user you are logged in as but effective user ID will be `0` (i-e root).
+    ///
+    /// ⚠️ It always returns `None` on Windows.
+    ///
+    /// ```no_run
+    /// use sysinfo::{Pid, ProcessExt, System, SystemExt};
+    ///
+    /// let mut s = System::new_all();
+    ///
+    /// if let Some(process) = s.process(Pid::from(1337)) {
+    ///     eprintln!("User id for process 1337: {:?}", process.effective_user_id());
+    /// }
+    /// ```
+    fn effective_user_id(&self) -> Option<&Uid>;
+
     /// Returns the process group ID of the process.
     ///
     /// ⚠️ It always returns `None` on Windows.
@@ -418,6 +438,24 @@ pub trait ProcessExt: Debug {
     /// ```
     fn group_id(&self) -> Option<Gid>;
 
+    /// Returns the effective group ID of the process.
+    ///
+    /// If you run something with `sudo`, the real group ID of the launched process will be the
+    /// primary group ID you are logged in as but effective group ID will be `0` (i-e root).
+    ///
+    /// ⚠️ It always returns `None` on Windows.
+    ///
+    /// ```no_run
+    /// use sysinfo::{Pid, ProcessExt, System, SystemExt};
+    ///
+    /// let mut s = System::new_all();
+    ///
+    /// if let Some(process) = s.process(Pid::from(1337)) {
+    ///     eprintln!("User id for process 1337: {:?}", process.effective_group_id());
+    /// }
+    /// ```
+    fn effective_group_id(&self) -> Option<Gid>;
+
     /// Wait for process termination.
     ///
     /// ```no_run
diff --git a/src/unknown/process.rs b/src/unknown/process.rs
index 61caae0..4e116d7 100644
--- a/src/unknown/process.rs
+++ b/src/unknown/process.rs
@@ -86,10 +86,18 @@ impl ProcessExt for Process {
         None
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        None
+    }
+
     fn group_id(&self) -> Option<Gid> {
         None
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        None
+    }
+
     fn wait(&self) {}
 
     fn session_id(&self) -> Option<Pid> {
diff --git a/src/windows/process.rs b/src/windows/process.rs
index 62a0490..6ec1d4b 100644
--- a/src/windows/process.rs
+++ b/src/windows/process.rs
@@ -525,10 +525,18 @@ impl ProcessExt for Process {
         self.user_id.as_ref()
     }
 
+    fn effective_user_id(&self) -> Option<&Uid> {
+        None
+    }
+
     fn group_id(&self) -> Option<Gid> {
         None
     }
 
+    fn effective_group_id(&self) -> Option<Gid> {
+        None
+    }
+
     fn wait(&self) {
         if let Some(handle) = self.get_handle() {
             while is_proc_running(handle) {


Test patch:
diff --git a/tests/process.rs b/tests/process.rs
index 3a4f916..71aa42c 100644
--- a/tests/process.rs
+++ b/tests/process.rs
@@ -615,3 +615,47 @@ fn test_process_cpu_usage() {
         assert!(process.cpu_usage() <= max_usage);
     }
 }
+
+#[test]
+fn test_process_creds() {
+    use sysinfo::{ProcessExt, System, SystemExt};
+
+    if !sysinfo::System::IS_SUPPORTED || cfg!(feature = "apple-sandbox") {
+        return;
+    }
+
+    let mut sys = System::new_all();
+    sys.refresh_all();
+
+    // Just ensure there is at least one process on the system whose credentials can be retrieved.
+    assert!(sys.processes().values().any(|process| {
+        if process.user_id().is_none() {
+            return false;
+        }
+
+        #[cfg(not(windows))]
+        {
+            if process.group_id().is_none()
+                || process.effective_user_id().is_none()
+                || process.effective_group_id().is_none()
+            {
+                return false;
+            }
+        }
+
+        true
+    }));
+
+    // On Windows, make sure no process has real group ID and no effective IDs.
+    #[cfg(windows)]
+    assert!(sys.processes().values().all(|process| {
+        if process.group_id().is_some()
+            || process.effective_user_id().is_some()
+            || process.effective_group_id().is_some()
+        {
+            return false;
+        }
+
+        true
+    }));
+}