[GH-ISSUE #4705] firejail tree+list flags bypass hidepid mounted /proc displaying details of other users sandboxes #2755

Open
opened 2026-05-05 09:25:09 -06:00 by gitea-mirror · 8 comments
Owner

Originally created by @rusty-snake on GitHub (Nov 21, 2021).
Original GitHub issue: https://github.com/netblue30/firejail/issues/4705

Originally assigned to: @netblue30, @smitsohu on GitHub.

Discussed in https://github.com/netblue30/firejail/discussions/4703

Originally posted by RyanOblivion112 November 21, 2021
Is this a bug or the intended behavior? It seems like it would be a bug since using firemon with/without the --tree/--list flags fails as expected with the message:
Error: /proc is mounted hidepid, you would need to be root to run this command

If it is intentional is there a setting that will "fix" it?

firejail version information(Fedora 34):

firejail version 0.9.64.4

Compile time support:
	- AppArmor support is disabled
	- AppImage support is enabled
	- chroot support is enabled
	- D-BUS proxy support is enabled
	- file and directory whitelisting support is enabled
	- file transfer support is enabled
	- firetunnel support is enabled
	- networking support is enabled
	- overlayfs support is disabled
	- private-home support is enabled
	- private-cache and tmpfs as user enabled
	- SELinux support is enabled
	- user namespace support is enabled
	- X11 sandboxing support is enabled
```</div>
Originally created by @rusty-snake on GitHub (Nov 21, 2021). Original GitHub issue: https://github.com/netblue30/firejail/issues/4705 Originally assigned to: @netblue30, @smitsohu on GitHub. ### Discussed in https://github.com/netblue30/firejail/discussions/4703 <div type='discussions-op-text'> <sup>Originally posted by **RyanOblivion112** November 21, 2021</sup> Is this a bug or the intended behavior? It _seems_ like it would be a bug since using firemon with/without the --tree/--list flags fails as expected with the message: `Error: /proc is mounted hidepid, you would need to be root to run this command` If it is intentional is there a setting that will "fix" it? firejail version information(Fedora 34): ``` firejail version 0.9.64.4 Compile time support: - AppArmor support is disabled - AppImage support is enabled - chroot support is enabled - D-BUS proxy support is enabled - file and directory whitelisting support is enabled - file transfer support is enabled - firetunnel support is enabled - networking support is enabled - overlayfs support is disabled - private-home support is enabled - private-cache and tmpfs as user enabled - SELinux support is enabled - user namespace support is enabled - X11 sandboxing support is enabled ```</div>
gitea-mirror added the
bug
security
labels 2026-05-05 09:25:09 -06:00
Author
Owner

@ghost commented on GitHub (Nov 24, 2021):

Something that may or may not be related is that I used to be unable to see firejail'd processes at all via top/ps(without root) when using a hidepid mounted /proc, and I can see them now. Unfortunately I can't recall if back then I was still able to see other users firejail processes when I ran firejail as a regular user with --tree/--list or not.

So it could be related to whatever change fixed that issue.

<!-- gh-comment-id:977741784 --> @ghost commented on GitHub (Nov 24, 2021): Something that may or may not be related is that I used to be unable to see firejail'd processes at all via top/ps(without root) when using a hidepid mounted /proc, and I can see them now. Unfortunately I can't recall if back then I was still able to see other users firejail processes when I ran firejail as a regular user with --tree/--list or not. So it _could_ be related to whatever change fixed that issue.
Author
Owner

@emerajid commented on GitHub (Apr 15, 2024):

I wrote some garbage that seems to work. Diff is against the 0.9.72 release. I know nothing of writing secure or reliable programs, so you'd think twice before using it.

diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c
index a379439..b287500 100644
--- a/src/firejail/sbox.c
+++ b/src/firejail/sbox.c
@@ -209,6 +209,26 @@ static int __attribute__((noreturn)) sbox_do_exec_v(unsigned filtermask, char *
 		if (setrlimit(RLIMIT_CORE, &tozero))
 			errExit("setrlimit");
 
+		if (env_index < 256) {
+			int l;
+			l = snprintf (NULL, 0, "FIREJAIL_RUID=%d", getuid ());
+			if (l < 0)
+				errExit ("snprintf");
+
+			char *envkey;
+			envkey = malloc (sizeof (char) * (l + 1));
+			if (!envkey)
+				errExit ("malloc");
+			memset (envkey, 0, sizeof (char) * (l + 1));
+
+			l = snprintf (envkey, sizeof (char) * (l + 1),
+					"FIREJAIL_RUID=%d", getuid ());
+			if (l < 0)
+				errExit ("snprintf");
+
+			new_environment[env_index++] = envkey;
+		}
+
 		// elevate privileges in order to get grsecurity working
 		if (setreuid(0, 0))
 			errExit("setreuid");
diff --git a/src/lib/pid.c b/src/lib/pid.c
index 5e9b20c..fec0cb0 100644
--- a/src/lib/pid.c
+++ b/src/lib/pid.c
@@ -26,6 +26,7 @@
 #include <pwd.h>
 #include <sys/ioctl.h>
 #include <dirent.h>
+#include <errno.h>
 
 #define PIDS_BUFLEN 4096
 //Process pids[max_pids];
@@ -315,6 +316,17 @@ void pid_read(pid_t mon_pid) {
 	struct dirent *entry;
 	char *end;
 	pid_t new_max_pids = 0;
+	int hide = pid_hidepid ();
+	char *realuid_str = getenv ("FIREJAIL_RUID");
+	int ruid = getuid ();
+	if (realuid_str) {
+		char *save[1];
+		ruid = (int) strtol (realuid_str, save, 10);
+		if (**save) {
+			ruid = getuid ();
+		}
+	}
+
 	while ((entry = readdir(dir))) {
 		pid_t pid = strtol(entry->d_name, &end, 10);
 		pid %= max_pids;
@@ -329,6 +341,35 @@ void pid_read(pid_t mon_pid) {
 		if (pid == 1)
 			continue;
 
+		// honour hidepid
+		if (hide) {
+			struct stat sb;
+			memset (&sb, 0, sizeof (struct stat));
+			int len;
+			len = snprintf (NULL, 0, "%s/%s", "/proc", entry->d_name);
+			if (len < 0)
+				errExit ("snprintf");
+			char procdir[len + 1];
+			memset (procdir, 0, len + 1);
+			len = snprintf (procdir, len + 1, "%s/%s", "/proc", entry->d_name);
+			if (len < 0)
+				errExit ("snprintf");
+
+			len = stat (procdir, &sb);
+			if (len == -1 && errno == ENOENT)
+				continue;
+			else if (len == -1)
+				errExit ("stat");
+
+			if (sb.st_uid != ruid) {
+				pids[pid].level = -1;
+				pids[pid].zombie = 0;
+				pids[pid].parent = 0;
+				pids[pid].uid = 0;
+				continue;
+			}
+		}
+
 		// open stat file
 		char *file;
 		if (asprintf(&file, "/proc/%u/status", pid) == -1)

<!-- gh-comment-id:2057967543 --> @emerajid commented on GitHub (Apr 15, 2024): I wrote some garbage that seems to work. Diff is against the 0.9.72 release. I know nothing of writing secure or reliable programs, so you'd think twice before using it. ``` diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c index a379439..b287500 100644 --- a/src/firejail/sbox.c +++ b/src/firejail/sbox.c @@ -209,6 +209,26 @@ static int __attribute__((noreturn)) sbox_do_exec_v(unsigned filtermask, char * if (setrlimit(RLIMIT_CORE, &tozero)) errExit("setrlimit"); + if (env_index < 256) { + int l; + l = snprintf (NULL, 0, "FIREJAIL_RUID=%d", getuid ()); + if (l < 0) + errExit ("snprintf"); + + char *envkey; + envkey = malloc (sizeof (char) * (l + 1)); + if (!envkey) + errExit ("malloc"); + memset (envkey, 0, sizeof (char) * (l + 1)); + + l = snprintf (envkey, sizeof (char) * (l + 1), + "FIREJAIL_RUID=%d", getuid ()); + if (l < 0) + errExit ("snprintf"); + + new_environment[env_index++] = envkey; + } + // elevate privileges in order to get grsecurity working if (setreuid(0, 0)) errExit("setreuid"); diff --git a/src/lib/pid.c b/src/lib/pid.c index 5e9b20c..fec0cb0 100644 --- a/src/lib/pid.c +++ b/src/lib/pid.c @@ -26,6 +26,7 @@ #include <pwd.h> #include <sys/ioctl.h> #include <dirent.h> +#include <errno.h> #define PIDS_BUFLEN 4096 //Process pids[max_pids]; @@ -315,6 +316,17 @@ void pid_read(pid_t mon_pid) { struct dirent *entry; char *end; pid_t new_max_pids = 0; + int hide = pid_hidepid (); + char *realuid_str = getenv ("FIREJAIL_RUID"); + int ruid = getuid (); + if (realuid_str) { + char *save[1]; + ruid = (int) strtol (realuid_str, save, 10); + if (**save) { + ruid = getuid (); + } + } + while ((entry = readdir(dir))) { pid_t pid = strtol(entry->d_name, &end, 10); pid %= max_pids; @@ -329,6 +341,35 @@ void pid_read(pid_t mon_pid) { if (pid == 1) continue; + // honour hidepid + if (hide) { + struct stat sb; + memset (&sb, 0, sizeof (struct stat)); + int len; + len = snprintf (NULL, 0, "%s/%s", "/proc", entry->d_name); + if (len < 0) + errExit ("snprintf"); + char procdir[len + 1]; + memset (procdir, 0, len + 1); + len = snprintf (procdir, len + 1, "%s/%s", "/proc", entry->d_name); + if (len < 0) + errExit ("snprintf"); + + len = stat (procdir, &sb); + if (len == -1 && errno == ENOENT) + continue; + else if (len == -1) + errExit ("stat"); + + if (sb.st_uid != ruid) { + pids[pid].level = -1; + pids[pid].zombie = 0; + pids[pid].parent = 0; + pids[pid].uid = 0; + continue; + } + } + // open stat file char *file; if (asprintf(&file, "/proc/%u/status", pid) == -1) ```
Author
Owner

@ghost commented on GitHub (Apr 20, 2024):

@smitsohu @topimiettinen Any opinions on the code quality of proposed fix from https://github.com/netblue30/firejail/issues/4705#issuecomment-2057967543?

<!-- gh-comment-id:2067734197 --> @ghost commented on GitHub (Apr 20, 2024): @smitsohu @topimiettinen Any opinions on the code quality of proposed fix from https://github.com/netblue30/firejail/issues/4705#issuecomment-2057967543?
Author
Owner

@topimiettinen commented on GitHub (Apr 21, 2024):

The approach is to filter /proc entries if the UIDs don't match. There should be a better way: don't allow joining namespaces of other users in the first place. I'm also wondering why drop_privs() keeps RUID==0.

<!-- gh-comment-id:2067987638 --> @topimiettinen commented on GitHub (Apr 21, 2024): The approach is to filter /proc entries if the UIDs don't match. There should be a better way: don't allow joining namespaces of other users in the first place. I'm also wondering why drop_privs() keeps RUID==0.
Author
Owner

@emerajid commented on GitHub (Apr 21, 2024):

don't allow joining namespaces of other users in the first place

Why exactly?

I'm also wondering why drop_privs() keeps RUID==0.

Because when hidepid is in action SBOX_ROOT flag is used and privileges are elevated to full root before executing firemon to provide compatibility with grsecurity. It's written in the source.

<!-- gh-comment-id:2068092384 --> @emerajid commented on GitHub (Apr 21, 2024): > don't allow joining namespaces of other users in the first place Why exactly? >I'm also wondering why drop_privs() keeps RUID==0. Because when hidepid is in action SBOX_ROOT flag is used and privileges are elevated to full root before executing firemon to provide compatibility with grsecurity. It's written in the source.
Author
Owner

@topimiettinen commented on GitHub (Apr 21, 2024):

don't allow joining namespaces of other users in the first place

Why exactly?

Your proposed method could be prone to TOCTOU issues.

I'm also wondering why drop_privs() keeps RUID==0.

Because when hidepid is in action SBOX_ROOT flag is used and privileges are elevated to full root before executing firemon to provide compatibility with grsecurity. It's written in the source.

Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default.

<!-- gh-comment-id:2068148729 --> @topimiettinen commented on GitHub (Apr 21, 2024): > > don't allow joining namespaces of other users in the first place > > Why exactly? Your proposed method could be prone to TOCTOU issues. > > I'm also wondering why drop_privs() keeps RUID==0. > > Because when hidepid is in action SBOX_ROOT flag is used and privileges are elevated to full root before executing firemon to provide compatibility with grsecurity. It's written in the source. Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default.
Author
Owner

@emerajid commented on GitHub (Apr 21, 2024):

Your proposed method could be prone to TOCTOU issues.

I have no clue about working with namespaces anyway, but if you expect anyone to look more closely at the issue, maybe you describe these issues in more detail? Maybe that will be helpful, because I have no idea how obvious are those TOCTOU issues to a competent developer.

Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default.

I'm afraid I have no idea, but if you would make it a compile option, you'd have to mess with Makefiles, which is something I can't help you with. Besides, I have no idea how many more code honors grsecurity, so this may appear to be too big to belong here.

Anyway if you expect anything useful to be done to address this issue, you should invite somebody who has a clue. I have only been able to write this "solution" because it is of "Hello, World!" complexity.

<!-- gh-comment-id:2068173924 --> @emerajid commented on GitHub (Apr 21, 2024): > Your proposed method could be prone to TOCTOU issues. I have no clue about working with namespaces anyway, but if you expect anyone to look more closely at the issue, maybe you describe these issues in more detail? Maybe that will be helpful, because I have no idea how obvious are those TOCTOU issues to a competent developer. > Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default. I'm afraid I have no idea, but if you would make it a compile option, you'd have to mess with Makefiles, which is something I can't help you with. Besides, I have no idea how many more code honors grsecurity, so this may appear to be too big to belong here. Anyway if you expect anything useful to be done to address this issue, you should invite somebody who has a clue. I have only been able to write this "solution" because it is of "Hello, World!" complexity.
Author
Owner

@topimiettinen commented on GitHub (Apr 21, 2024):

Your proposed method could be prone to TOCTOU issues.

I have no clue about working with namespaces anyway, but if you expect anyone to look more closely at the issue, maybe you describe these issues in more detail? Maybe that will be helpful, because I have no idea how obvious are those TOCTOU issues to a competent developer.[

Here's Wikipedia entry for TOCTOU.

Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default.

I'm afraid I have no idea, but if you would make it a compile option, you'd have to mess with Makefiles, which is something I can't help you with. Besides, I have no idea how many more code honors grsecurity, so this may appear to be too big to belong here.

Right, this was more of a side note, something to be fixed outside of this PR (unless it's indeed RUID which gives access).

Anyway if you expect anything useful to be done to address this issue, you should invite somebody who has a clue. I have only been able to write this "solution" because it is of "Hello, World!" complexity.

Thanks for your contribution. Nice thing with Github etc. is that many solutions can be discussed before settling on one.

<!-- gh-comment-id:2068190540 --> @topimiettinen commented on GitHub (Apr 21, 2024): > > Your proposed method could be prone to TOCTOU issues. > > I have no clue about working with namespaces anyway, but if you expect anyone to look more closely at the issue, maybe you describe these issues in more detail? Maybe that will be helpful, because I have no idea how obvious are those TOCTOU issues to a competent developer.[ Here's [Wikipedia entry](https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use) for TOCTOU. > > Is grsecurity still a thing? This kind of weakening should be a compile option, not enabled by default. > > I'm afraid I have no idea, but if you would make it a compile option, you'd have to mess with Makefiles, which is something I can't help you with. Besides, I have no idea how many more code honors grsecurity, so this may appear to be too big to belong here. Right, this was more of a side note, something to be fixed outside of this PR (unless it's indeed RUID which gives access). > > Anyway if you expect anything useful to be done to address this issue, you should invite somebody who has a clue. I have only been able to write this "solution" because it is of "Hello, World!" complexity. Thanks for your contribution. Nice thing with Github etc. is that many solutions can be discussed before settling on one.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#2755
No description provided.