hardening: run more code unprivileged

4 files changed, 71 insertions, 69 deletions

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 01ddf2a14..5291361c8 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -521,6 +521,7 @@ void logsignal(int s);
 void logmsg(const char *msg);
 void logargs(int argc, char **argv) ;
 void logerr(const char *msg);
+void set_nice(int inc);
 int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode);
 void copy_file_as_user(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode);
 void copy_file_from_user_to_root(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode);
diff --git a/src/firejail/join.c b/src/firejail/join.c
index 3372c8dc1..46dae0271 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -100,9 +100,6 @@ static void extract_command(int argc, char **argv, int index) {
 
         // build command
         build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index);
-
-        if (arg_debug)
-                printf("Extracted command #%s#\n", cfg.command_line);
 }
 
 static void extract_nogroups(pid_t pid) {
@@ -290,11 +287,8 @@ pid_t switch_to_child(pid_t pid) {
 
 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);
 
+        pid_t parent = pid;
         // in case the pid is that of a firejail process, use the pid of the first child process
         pid = switch_to_child(pid);
 
@@ -374,19 +368,15 @@ void join(pid_t pid, int argc, char **argv, int index) {
                 EUID_USER();
                 if (chdir("/") < 0)
                         errExit("chdir");
-                if (homedir) {
+                if (cfg.homedir) {
                         struct stat s;
-                        if (stat(homedir, &s) == 0) {
+                        if (stat(cfg.homedir, &s) == 0) {
                                 /* coverity[toctou] */
-                                if (chdir(homedir) < 0)
+                                if (chdir(cfg.homedir) < 0)
                                         errExit("chdir");
                         }
                 }
 
-                // set cpu affinity
-                if (cfg.cpus)   // not available for uid 0
-                        set_cpu_affinity();
-
                 // set caps filter
                 EUID_ROOT();
                 if (apply_caps == 1)    // not available for uid 0
@@ -417,33 +407,6 @@ void join(pid_t pid, int argc, char **argv, int index) {
                 }
 
                 EUID_USER();
-                // set nice
-                if (arg_nice) {
-                        errno = 0;
-                        int rv = nice(cfg.nice);
-                        (void) rv;
-                        if (errno) {
-                                fwarning("cannot set nice value\n");
-                                errno = 0;
-                        }
-                }
-
-                // set environment, add x11 display
-                env_defaults();
-                if (display) {
-                        char *display_str;
-                        if (asprintf(&display_str, ":%d", display) == -1)
-                                errExit("asprintf");
-                        setenv("DISPLAY", display_str, 1);
-                        free(display_str);
-                }
-
-                if (cfg.command_line == NULL) {
-                        assert(cfg.shell);
-                        cfg.command_line = cfg.shell;
-                        cfg.window_title = cfg.shell;
-                }
-
                 int cwd = 0;
                 if (cfg.cwd) {
                         if (chdir(cfg.cwd) == 0)
@@ -463,8 +426,38 @@ void join(pid_t pid, int argc, char **argv, int index) {
                         }
                 }
 
+                // drop privileges
                 drop_privs(arg_nogroups);
-                prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); // kill the child in case the parent died
+
+                // kill the child in case the parent died
+                prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
+
+                extract_command(argc, argv, index);
+                if (cfg.command_line == NULL) {
+                        assert(cfg.shell);
+                        cfg.command_line = cfg.shell;
+                        cfg.window_title = cfg.shell;
+                }
+                if (arg_debug)
+                        printf("Extracted command #%s#\n", cfg.command_line);
+
+                // set cpu affinity
+                if (cfg.cpus)   // not available for uid 0
+                        set_cpu_affinity();
+
+                // set nice value
+                if (arg_nice)
+                        set_nice(cfg.nice);
+
+                // add x11 display
+                if (display) {
+                        char *display_str;
+                        if (asprintf(&display_str, ":%d", display) == -1)
+                                errExit("asprintf");
+                        setenv("DISPLAY", display_str, 1);
+                        free(display_str);
+                }
+
                 start_application(0, NULL);
 
                 // it will never get here!!!
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 9bb8e545c..2b5d30158 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -1038,17 +1038,6 @@ int sandbox(void* sandbox_arg) {
                 }
         }
 
-        // set nice
-        if (arg_nice) {
-                errno = 0;
-                int rv = nice(cfg.nice);
-                (void) rv;
-                if (errno) {
-                        fwarning("cannot set nice value\n");
-                        errno = 0;
-                }
-        }
-
         EUID_ROOT();
         // clean /tmp/.X11-unix sockets
         fs_x11();
@@ -1067,17 +1056,11 @@ int sandbox(void* sandbox_arg) {
         // set capabilities
         set_caps();
 
-        // set cpu affinity
-        if (cfg.cpus) {
-                save_cpu(); // save cpu affinity mask to CPU_CFG file
-                EUID_USER();
-                set_cpu_affinity();
-                EUID_ROOT();
-        }
+        // save cpu affinity mask to CPU_CFG file
+        save_cpu();
 
         // save cgroup in CGROUP_CFG file
-        if (cfg.cgroup)
-                save_cgroup();
+        save_cgroup();
 
         // set seccomp
 #ifdef HAVE_SECCOMP
@@ -1125,7 +1108,7 @@ int sandbox(void* sandbox_arg) {
         // to --join
         //****************************************
 
-        FILE *fp = create_ready_for_join_file();
+        FILE *rj = create_ready_for_join_file();
 
         //****************************************
         // create a new user namespace
@@ -1175,10 +1158,23 @@ int sandbox(void* sandbox_arg) {
         }
 
         //****************************************
-        // drop privileges, fork the application and monitor it
+        // drop privileges
         //****************************************
         drop_privs(arg_nogroups);
-        prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); // kill the sandbox in case the parent died
+
+        // kill the sandbox in case the parent died
+        prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
+
+        //****************************************
+        // set cpu affinity
+        //****************************************
+
+        if (cfg.cpus)
+                set_cpu_affinity();
+
+        //****************************************
+        // fork the application and monitor it
+        //****************************************
         pid_t app_pid = fork();
         if (app_pid == -1)
                 errExit("fork");
@@ -1196,13 +1192,15 @@ int sandbox(void* sandbox_arg) {
                                 printf("AppArmor enabled\n");
                 }
 #endif
-                // set rlimits
+                // set nice and rlimits
+                if (arg_nice)
+                        set_nice(cfg.nice);
                 set_rlimits();
-                // start app
-                start_application(0, fp);
+
+                start_application(0, rj);
         }
 
-        fclose(fp);
+        fclose(rj);
 
         int status = monitor_application(app_pid);      // monitor application
         flush_stdin();
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 46b392eed..3e2cd13d5 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -250,6 +250,16 @@ void logerr(const char *msg) {
         closelog();
 }
 
+
+void set_nice(int inc) {
+        errno = 0;
+        int rv = nice(inc);
+        (void) rv;
+        if (errno)
+                fwarning("cannot set nice value\n");
+}
+
+
 static int copy_file_by_fd(int src, int dst) {
         assert(src >= 0);
         assert(dst >= 0);