fix issue with join option

5 files changed, 131 insertions, 15 deletions

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 5ce380503..348e4f6ab 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -100,6 +100,7 @@
 #define RUN_FSLOGGER_FILE               "/run/firejail/mnt/fslogger"
 #define RUN_UMASK_FILE          "/run/firejail/mnt/umask"
 #define RUN_OVERLAY_ROOT        "/run/firejail/mnt/oroot"
+#define RUN_READY_FOR_JOIN      "/run/firejail/mnt/ready-for-join"
 
 
 // profiles
@@ -405,7 +406,7 @@ char *guess_shell(void);
 
 // sandbox.c
 int sandbox(void* sandbox_arg);
-void start_application(int no_sandbox);
+void start_application(int no_sandbox, FILE *fp);
 
 // network_main.c
 void net_configure_sandbox_ip(Bridge *br);
diff --git a/src/firejail/join.c b/src/firejail/join.c
index 729c7f797..17191d9a9 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -212,8 +212,10 @@ static void extract_umask(pid_t pid) {
 
         FILE *fp = fopen(fname, "re");
         free(fname);
-        if (!fp)
-                return;
+        if (!fp) {
+                fprintf(stderr, "Error: cannot open umask file\n");
+                exit(1);
+        }
         if (fscanf(fp, "%3o", &orig_umask) < 1) {
                 fprintf(stderr, "Error: cannot read umask\n");
                 exit(1);
@@ -221,6 +223,43 @@ static void extract_umask(pid_t pid) {
         fclose(fp);
 }
 
+static void 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.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;
@@ -231,17 +270,53 @@ void join(pid_t pid, int argc, char **argv, int index) {
 
         // if the pid is that of a firejail  process, use the pid of the first child process
         EUID_ROOT();
+        errno = 0;
         char *comm = pid_proc_comm(pid);
+        if (!comm) {
+                if (errno == ENOENT) {
+                        fprintf(stderr, "Error: cannot find process with id %d\n", pid);
+                        exit(1);
+                }
+                else {
+                        fprintf(stderr, "Error: cannot read /proc/%d/comm\n", pid);
+                        exit(1);
+                }
+        }
         EUID_USER();
-        if (comm) {
-                if (strcmp(comm, "firejail") == 0) {
-                        pid_t child;
-                        if (find_child(pid, &child) == 0) {
-                                pid = child;
-                                fmessage("Switching to pid %u, the first child process inside the sandbox\n", (unsigned) pid);
+        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");
+                        exit(1);
+                }
+                fmessage("Switching to pid %u, the first child process inside the sandbox\n", (unsigned) child);
+                pid = child;
+        }
+        free(comm);
+
+        // pid must belong to a valid firejail sandbox
+        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;
+        for (int 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;
                 }
-                free(comm);
+                current = next;
         }
 
         // check privileges for non-root users
@@ -406,7 +481,7 @@ void join(pid_t pid, int argc, char **argv, int index) {
                 }
 
                 drop_privs(arg_nogroups);
-                start_application(0);
+                start_application(0, NULL);
 
                 // it will never get here!!!
         }
diff --git a/src/firejail/no_sandbox.c b/src/firejail/no_sandbox.c
index 5bd3f7e09..7c5cc1df9 100644
--- a/src/firejail/no_sandbox.c
+++ b/src/firejail/no_sandbox.c
@@ -233,5 +233,5 @@ void run_no_sandbox(int argc, char **argv) {
 
         arg_quiet = 1;
 
-        start_application(1);
+        start_application(1, NULL);
 }
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 2c28aa24b..5d9526b4c 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -139,6 +139,18 @@ void save_umask(void) {
         }
 }
 
+static FILE *create_ready_for_join_file(void) {
+        FILE *fp = fopen(RUN_READY_FOR_JOIN, "wxe");
+        if (fp) {
+                ASSERT_PERMS_STREAM(fp, 0, 0, 0644);
+                return fp;
+        }
+        else {
+                fprintf(stderr, "Error: cannot create %s\n", RUN_READY_FOR_JOIN);
+                exit(1);
+        }
+}
+
 static void sandbox_if_up(Bridge *br) {
         assert(br);
         if (!br->configured)
@@ -374,7 +386,7 @@ static int ok_to_run(const char *program) {
         return 0;
 }
 
-void start_application(int no_sandbox) {
+void start_application(int no_sandbox, FILE *fp) {
         // set environment
         if (no_sandbox == 0) {
                 env_defaults();
@@ -393,6 +405,11 @@ void start_application(int no_sandbox) {
         //****************************************
         if (arg_audit) {
                 assert(arg_audit_prog);
+
+                if (fp) {
+                        fprintf(fp, "ready\n");
+                        fclose(fp);
+                }
 #ifdef HAVE_GCOV
                 __gcov_dump();
 #endif
@@ -423,6 +440,11 @@ void start_application(int no_sandbox) {
                         print_time();
 
                 int rv = ok_to_run(cfg.original_argv[cfg.original_program_index]);
+
+                if (fp) {
+                        fprintf(fp, "ready\n");
+                        fclose(fp);
+                }
 #ifdef HAVE_GCOV
                 __gcov_dump();
 #endif
@@ -479,6 +501,11 @@ void start_application(int no_sandbox) {
 
                 if (!arg_command && !arg_quiet)
                         print_time();
+
+                if (fp) {
+                        fprintf(fp, "ready\n");
+                        fclose(fp);
+                }
 #ifdef HAVE_GCOV
                 __gcov_dump();
 #endif
@@ -1069,6 +1096,13 @@ int sandbox(void* sandbox_arg) {
 #endif
 
         //****************************************
+        // communicate progress of sandbox set up
+        // to --join
+        //****************************************
+
+        FILE *fp = create_ready_for_join_file();
+
+        //****************************************
         // create a new user namespace
         //     - too early to drop privileges
         //****************************************
@@ -1133,9 +1167,11 @@ int sandbox(void* sandbox_arg) {
 #endif
 
                 prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); // kill the child in case the parent died
-                start_application(0);   // start app
+                start_application(0, fp);       // start app
         }
 
+        fclose(fp);
+
         int status = monitor_application(app_pid);      // monitor application
         flush_stdin();
 
diff --git a/src/lib/common.c b/src/lib/common.c
index fa988446b..d6dd43c4b 100644
--- a/src/lib/common.c
+++ b/src/lib/common.c
@@ -129,7 +129,7 @@ char *pid_proc_comm(const pid_t pid) {
         // open /proc/pid/cmdline file
         char *fname;
         int fd;
-        if (asprintf(&fname, "/proc/%d//comm", pid) == -1)
+        if (asprintf(&fname, "/proc/%d/comm", pid) == -1)
                 return NULL;
         if ((fd = open(fname, O_RDONLY)) < 0) {
                 free(fname);
@@ -154,6 +154,8 @@ char *pid_proc_comm(const pid_t pid) {
 
         // return a malloc copy of the command line
         char *rv = strdup(buffer);
+        if (!rv)
+                return NULL;
         if (strlen(rv) == 0) {
                 free(rv);
                 return NULL;
@@ -192,6 +194,8 @@ char *pid_proc_cmdline(const pid_t pid) {
 
         // return a malloc copy of the command line
         char *rv = strdup((char *) buffer);
+        if (!rv)
+                return NULL;
         if (strlen(rv) == 0) {
                 free(rv);
                 return NULL;