refactor, check the sandbox status for all join options

12 files changed, 145 insertions, 156 deletions

diff --git a/src/firejail/bandwidth.c b/src/firejail/bandwidth.c
index 49a94186c..d7764682a 100644
--- a/src/firejail/bandwidth.c
+++ b/src/firejail/bandwidth.c
@@ -333,6 +333,11 @@ void bandwidth_pid(pid_t pid, const char *command, const char *dev, int down, in
                 exit(1);
         }
 
+        if (invalid_sandbox(child)) {
+                fprintf(stderr, "Error: cannot join the network namespace\n");
+                exit(1);
+        }
+
         EUID_ROOT();
         if (join_namespace(child, "net")) {
                 fprintf(stderr, "Error: cannot join the network namespace\n");
diff --git a/src/firejail/caps.c b/src/firejail/caps.c
index dae45d9df..bd3b5e229 100644
--- a/src/firejail/caps.c
+++ b/src/firejail/caps.c
@@ -401,18 +401,13 @@ errexit:
 void caps_print_filter(pid_t pid) {
         EUID_ASSERT();
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();    // grsecurity
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();    // grsecurity
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/cpu.c b/src/firejail/cpu.c
index 8f72fb69e..a92562e67 100644
--- a/src/firejail/cpu.c
+++ b/src/firejail/cpu.c
@@ -165,18 +165,13 @@ static void print_cpu(int pid) {
 void cpu_print_filter(pid_t pid) {
         EUID_ASSERT();
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();    // grsecurity
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();    // grsecurity
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 348e4f6ab..906ec6307 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -478,6 +478,7 @@ void usage(void);
 
 // join.c
 void join(pid_t pid, int argc, char **argv, int index);
+pid_t switch_to_child(pid_t pid);
 
 // shutdown.c
 void shut(pid_t pid);
@@ -538,6 +539,7 @@ unsigned extract_timeout(const char *str);
 void disable_file_or_dir(const char *fname);
 void disable_file_path(const char *path, const char *file);
 int safe_fd(const char *path, int flags);
+int invalid_sandbox(const pid_t pid);
 
 // Get info regarding the last kernel mount operation from /proc/self/mountinfo
 // The return value points to a static area, and will be overwritten by subsequent calls.
diff --git a/src/firejail/fs_logger.c b/src/firejail/fs_logger.c
index 93f28a26b..bf3ac26be 100644
--- a/src/firejail/fs_logger.c
+++ b/src/firejail/fs_logger.c
@@ -120,18 +120,13 @@ void fs_logger_change_owner(void) {
 void fs_logger_print_log(pid_t pid) {
         EUID_ASSERT();
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/join.c b/src/firejail/join.c
index 910e97d07..cdd95b6a8 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -223,52 +223,7 @@ static void extract_umask(pid_t pid) {
         fclose(fp);
 }
 
-static void check_ready_for_join(pid_t pid) {
-        char *fname;
-        if (asprintf(&fname, "/proc/%d/root%s", pid, RUN_READY_FOR_JOIN) == -1)
-                errExit("asprintf");
-
-        // check if file "ready-for-join" exists
-        EUID_ROOT();
-        FILE *fp = fopen(fname, "re");
-        EUID_USER();
-        if (!fp)
-                goto errexit;
-        int fd = fileno(fp);
-        if (fd == -1)
-                errExit("fileno");
-        struct stat s;
-        if (fstat(fd, &s) == -1)
-                errExit("fstat");
-        if (!S_ISREG(s.st_mode) || s.st_uid != 0)
-                goto errexit;
-        // check if it is non-empty
-        char buf[BUFLEN];
-        if (fgets(buf, BUFLEN, fp) == NULL)
-                goto errexit;
-        // confirm "ready" string was written
-        if (strncmp(buf, "ready\n", 6) != 0)
-                goto errexit;
-        fclose(fp);
-        free(fname);
-        return;
-
-errexit:
-        fprintf(stderr, "Error: no valid sandbox\n");
-        exit(1);
-}
-
-
-
-void join(pid_t pid, int argc, char **argv, int index) {
-        EUID_ASSERT();
-        char *homedir = cfg.homedir;
-        pid_t parent = pid;
-
-        extract_command(argc, argv, index);
-        signal (SIGTERM, signal_handler);
-
-        // if the pid is that of a firejail  process, use the pid of the first child process
+pid_t switch_to_child(pid_t pid) {
         EUID_ROOT();
         errno = 0;
         char *comm = pid_proc_comm(pid);
@@ -278,7 +233,7 @@ void join(pid_t pid, int argc, char **argv, int index) {
                         exit(1);
                 }
                 else {
-                        fprintf(stderr, "Error: cannot read /proc/%d/comm\n", pid);
+                        fprintf(stderr, "Error: cannot read /proc file\n");
                         exit(1);
                 }
         }
@@ -286,38 +241,33 @@ void join(pid_t pid, int argc, char **argv, int index) {
         if (strcmp(comm, "firejail") == 0) {
                 pid_t child;
                 if (find_child(pid, &child) == 1) {
-                        fprintf(stderr, "Error: found firejail process without a child process\n");
+                        fprintf(stderr, "Error: no valid sandbox\n");
                         exit(1);
                 }
                 fmessage("Switching to pid %u, the first child process inside the sandbox\n", (unsigned) child);
                 pid = child;
         }
         free(comm);
+        return pid;
+}
 
-        // pid must belong to a valid firejail sandbox
-        check_ready_for_join(pid);
-
-        // walk down the process tree a few nodes, there should be no firejail leaf
-#define MAXNODES 4
-        pid_t current = pid, next;
-        int i;
-        for (i = 0; i < MAXNODES; i++) {
-                if (find_child(current, &next) == 1) {
-                        EUID_ROOT();
-                        comm = pid_proc_comm(current);
-                        EUID_USER();
-                        if (!comm) {
-                                fprintf(stderr, "Error: cannot read /proc/%d/comm\n", current);
-                                exit(1);
-                        }
-                        if (strcmp(comm, "firejail") == 0) {
-                                fprintf(stderr, "Error: found firejail process without a child process\n");
-                                exit(1);
-                        }
-                        free(comm);
-                        break;
-                }
-                current = next;
+
+
+void join(pid_t pid, int argc, char **argv, int index) {
+        EUID_ASSERT();
+        char *homedir = cfg.homedir;
+        pid_t parent = pid;
+
+        extract_command(argc, argv, index);
+        signal (SIGTERM, signal_handler);
+
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index 601cab4f8..51370da60 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -212,18 +212,13 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
         EUID_ASSERT();
         assert(path1);
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/netfilter.c b/src/firejail/netfilter.c
index 11c7e9669..8fbd11bba 100644
--- a/src/firejail/netfilter.c
+++ b/src/firejail/netfilter.c
@@ -175,6 +175,11 @@ void netfilter_print(pid_t pid, int ipv6) {
                 exit(1);
         }
 
+        if (invalid_sandbox(child)) {
+                fprintf(stderr, "Error: cannot join the network namespace\n");
+                exit(1);
+        }
+
         EUID_ROOT();
         if (join_namespace(child, "net")) {
                 fprintf(stderr, "Error: cannot join the network namespace\n");
diff --git a/src/firejail/network_main.c b/src/firejail/network_main.c
index 1a40e8a0f..e3c750767 100644
--- a/src/firejail/network_main.c
+++ b/src/firejail/network_main.c
@@ -269,18 +269,13 @@ void net_dns_print(pid_t pid) {
         EUID_ASSERT();
         // drop privileges - will not be able to read /etc/resolv.conf for --noroot option
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/protocol.c b/src/firejail/protocol.c
index eb3763253..9989ddb68 100644
--- a/src/firejail/protocol.c
+++ b/src/firejail/protocol.c
@@ -64,18 +64,13 @@ void protocol_print_filter(pid_t pid) {
 
         (void) pid;
 #ifdef SYS_socket
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/seccomp.c b/src/firejail/seccomp.c
index 3da0206e1..7be7b3950 100644
--- a/src/firejail/seccomp.c
+++ b/src/firejail/seccomp.c
@@ -295,18 +295,13 @@ int seccomp_filter_keep(void) {
 void seccomp_print_filter(pid_t pid) {
         EUID_ASSERT();
 
-        // if the pid is that of a firejail  process, use the pid of the first child process
-        EUID_ROOT();
-        char *comm = pid_proc_comm(pid);
-        EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                        }
-                }
-                free(comm);
+        // in case the pid is that of a firejail process, use the pid of the first child process
+        pid = switch_to_child(pid);
+
+        // now check if the pid belongs to a firejail sandbox
+        if (invalid_sandbox(pid)) {
+                fprintf(stderr, "Error: no valid sandbox\n");
+                exit(1);
         }
 
         // check privileges for non-root users
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 5738e7cf8..c15e3b691 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -1090,3 +1090,65 @@ errexit:
         fprintf(stderr, "Error: cannot open \"%s\", invalid filename\n", path);
         exit(1);
 }
+
+
+// return 1 if the sandbox identified by pid is not fully set up yet or if
+// it is no firejail sandbox at all, return 0 if the sandbox is complete
+int invalid_sandbox(const pid_t pid) {
+        // check if a file "ready-for-join" exists
+        char *fname;
+        if (asprintf(&fname, "/proc/%d/root%s", pid, RUN_READY_FOR_JOIN) == -1)
+                errExit("asprintf");
+        EUID_ROOT();
+        FILE *fp = fopen(fname, "re");
+        EUID_USER();
+        free(fname);
+        if (!fp)
+                return 1;
+        // regular file owned by root
+        int fd = fileno(fp);
+        if (fd == -1)
+                errExit("fileno");
+        struct stat s;
+        if (fstat(fd, &s) == -1)
+                errExit("fstat");
+        if (!S_ISREG(s.st_mode) || s.st_uid != 0) {
+                fclose(fp);
+                return 1;
+        }
+        // check if it is non-empty
+        char buf[BUFLEN];
+        if (fgets(buf, BUFLEN, fp) == NULL) {
+                fclose(fp);
+                return 1;
+        }
+        fclose(fp);
+        // confirm "ready" string was written
+        if (strncmp(buf, "ready\n", 6) != 0)
+                return 1;
+
+        // walk down the process tree a few nodes, there should be no firejail leaf
+#define MAXNODES 5
+        pid_t current = pid, next;
+        int i;
+        for (i = 0; i < MAXNODES; i++) {
+                if (find_child(current, &next) == 1) {
+                        EUID_ROOT();
+                        char *comm = pid_proc_comm(current);
+                        EUID_USER();
+                        if (!comm) {
+                                fprintf(stderr, "Error: cannot read /proc file\n");
+                                exit(1);
+                        }
+                        if (strcmp(comm, "firejail") == 0) {
+                                free(comm);
+                                return 1;
+                        }
+                        free(comm);
+                        break;
+                }
+                current = next;
+        }
+
+        return 0;
+}