From 1d18b57644b0429437deb94c056ca36162d79f7c Mon Sep 17 00:00:00 2001 From: smitsohu Date: Thu, 30 Aug 2018 00:06:12 +0200 Subject: reject chroot if it is world-writable, related enhancements --- src/firejail/fs.c | 60 +++++++++++++++++++++++++++++++++++++---------------- src/firejail/main.c | 2 +- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 825f004cc..ed0131b1d 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -1160,7 +1160,8 @@ void fs_check_chroot_dir(const char *rootdir) { exit(1); } - // rootdir has to be owned by root + // rootdir has to be owned by root and is not allowed to be world-writable + // we checked already if it is a directory if (stat(rootdir, &s) != 0) { fprintf(stderr, "Error: cannot find chroot directory\n"); exit(1); @@ -1169,6 +1170,10 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: chroot directory should be owned by root\n"); exit(1); } + if ((S_IWOTH & s.st_mode) != 0) { + fprintf(stderr, "Error: chroot directory is not allowed to be world-writable\n"); + exit(1); + } // check /dev if (asprintf(&name, "%s/dev", rootdir) == -1) @@ -1177,8 +1182,8 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /dev in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /dev directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /dev should be a directory owned by root\n"); exit(1); } free(name); @@ -1190,8 +1195,8 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /var/tmp in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /var/tmp directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /var/tmp should be a directory owned by root\n"); exit(1); } free(name); @@ -1203,8 +1208,8 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /proc in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /proc directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /proc should be a directory owned by root\n"); exit(1); } free(name); @@ -1216,8 +1221,8 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /tmp in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /tmp directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /tmp should be a directory owned by root\n"); exit(1); } free(name); @@ -1229,8 +1234,12 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /etc in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /etc directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /etc should be a directory owned by root\n"); + exit(1); + } + if ((S_IWOTH & s.st_mode) != 0) { + fprintf(stderr, "Error: chroot /etc is not allowed to be world-writable\n"); exit(1); } free(name); @@ -1272,8 +1281,8 @@ void fs_check_chroot_dir(const char *rootdir) { fprintf(stderr, "Error: cannot find /tmp/.X11-unix in chroot directory\n"); exit(1); } - if (s.st_uid != 0) { - fprintf(stderr, "Error: chroot /tmp/.X11-unix directory should be owned by root\n"); + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /tmp/.X11-unix should be a directory owned by root\n"); exit(1); } free(name); @@ -1308,16 +1317,29 @@ void fs_chroot(const char *rootdir) { // some older distros don't have a /run directory // create one by default - // create /run/firejail directory in chroot char *rundir; if (asprintf(&rundir, "%s/run", rootdir) == -1) errExit("asprintf"); - if (is_link(rundir)) { - fprintf(stderr, "Error: invalid run directory inside chroot\n"); - exit(1); + struct stat s; + if (lstat(rundir, &s) == 0) { + if (S_ISLNK(s.st_mode)) { + fprintf(stderr, "Error: chroot /run is a symbolic link\n"); + exit(1); + } + if (!S_ISDIR(s.st_mode) || s.st_uid != 0) { + fprintf(stderr, "Error: chroot /run should be a directory owned by root\n"); + exit(1); + } + if ((S_IWOTH & s.st_mode) != 0) { + fprintf(stderr, "Error: chroot /run is not allowed to be world-writable\n"); + exit(1); + } } - create_empty_dir_as_root(rundir, 0755); + else + create_empty_dir_as_root(rundir, 0755); free(rundir); + + // create /run/firejail directory in chroot if (asprintf(&rundir, "%s/run/firejail", rootdir) == -1) errExit("asprintf"); create_empty_dir_as_root(rundir, 0755); @@ -1329,6 +1351,7 @@ void fs_chroot(const char *rootdir) { create_empty_dir_as_root(rundir, 0755); if (mount(RUN_MNT_DIR, rundir, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); + free(rundir); // copy /etc/resolv.conf in chroot directory char *fname; @@ -1339,6 +1362,7 @@ void fs_chroot(const char *rootdir) { unlink(fname); if (copy_file("/etc/resolv.conf", fname, 0, 0, 0644) == -1) // root needed fwarning("/etc/resolv.conf not initialized\n"); + free(fname); // chroot into the new directory #ifdef HAVE_GCOV diff --git a/src/firejail/main.c b/src/firejail/main.c index ba952b1cb..435c04d76 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -1502,7 +1502,7 @@ int main(int argc, char **argv) { // check chroot dirname exists if (strstr(cfg.chrootdir, "..") || !is_dir(cfg.chrootdir) || is_link(cfg.chrootdir)) { - fprintf(stderr, "Error: invalid directory %s\n", cfg.chrootdir); + fprintf(stderr, "Error: invalid chroot directory %s\n", cfg.chrootdir); return 1; } -- cgit v1.2.3-70-g09d2