From 9aa81442afc6e00ca177bf0e3e7a025195102f7d Mon Sep 17 00:00:00 2001 From: netblue30 Date: Tue, 10 Jan 2017 10:07:20 -0500 Subject: security fix --- src/firejail/firejail.h | 2 + src/firejail/fs_home.c | 148 +++++++++++----------------------------------- src/firejail/pulseaudio.c | 1 + src/firejail/util.c | 46 ++++++++++++++ 4 files changed, 85 insertions(+), 112 deletions(-) (limited to 'src') diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 36cf47435..a8208233f 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -450,6 +450,8 @@ void logmsg(const char *msg); void logargs(int argc, char **argv) ; void logerr(const char *msg); 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 touch_file_as_user(const char *fname, uid_t uid, gid_t gid, mode_t mode); int is_dir(const char *fname); int is_link(const char *fname); char *line_remove_spaces(const char *buf); diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index 4de082b06..e4b19d5cc 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -42,19 +42,17 @@ static void skel(const char *homedir, uid_t u, gid_t g) { // don't copy it if we already have the file if (stat(fname, &s) == 0) return; + if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + fprintf(stderr, "Error: invalid %s file\n", fname); + exit(1); + } if (stat("/etc/skel/.zshrc", &s) == 0) { - if (copy_file("/etc/skel/.zshrc", fname, u, g, 0644) == 0) { - fs_logger("clone /etc/skel/.zshrc"); - } + copy_file_as_user("/etc/skel/.zshrc", fname, u, g, 0644); + fs_logger("clone /etc/skel/.zshrc"); } - else { // - FILE *fp = fopen(fname, "w"); - if (fp) { - fprintf(fp, "\n"); - SET_PERMS_STREAM(fp, u, g, S_IRUSR | S_IWUSR); - fclose(fp); - fs_logger2("touch", fname); - } + else { + touch_file_as_user(fname, u, g, 0644); + fs_logger2("touch", fname); } free(fname); } @@ -64,23 +62,21 @@ static void skel(const char *homedir, uid_t u, gid_t g) { if (asprintf(&fname, "%s/.cshrc", homedir) == -1) errExit("asprintf"); struct stat s; + // don't copy it if we already have the file if (stat(fname, &s) == 0) return; + if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + fprintf(stderr, "Error: invalid %s file\n", fname); + exit(1); + } if (stat("/etc/skel/.cshrc", &s) == 0) { - if (copy_file("/etc/skel/.cshrc", fname, u, g, 0644) == 0) { - fs_logger("clone /etc/skel/.cshrc"); - } + copy_file_as_user("/etc/skel/.cshrc", fname, u, g, 0644); + fs_logger("clone /etc/skel/.cshrc"); } - else { // - /* coverity[toctou] */ - FILE *fp = fopen(fname, "w"); - if (fp) { - fprintf(fp, "\n"); - SET_PERMS_STREAM(fp, u, g, S_IRUSR | S_IWUSR); - fclose(fp); - fs_logger2("touch", fname); - } + else { + touch_file_as_user(fname, u, g, 0644); + fs_logger2("touch", fname); } free(fname); } @@ -93,10 +89,13 @@ static void skel(const char *homedir, uid_t u, gid_t g) { // don't copy it if we already have the file if (stat(fname, &s) == 0) return; + if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + fprintf(stderr, "Error: invalid %s file\n", fname); + exit(1); + } if (stat("/etc/skel/.bashrc", &s) == 0) { - if (copy_file("/etc/skel/.bashrc", fname, u, g, 0644) == 0) { - fs_logger("clone /etc/skel/.bashrc"); - } + copy_file_as_user("/etc/skel/.bashrc", fname, u, g, 0644); + fs_logger("clone /etc/skel/.bashrc"); } free(fname); } @@ -106,7 +105,7 @@ static int store_xauthority(void) { // put a copy of .Xauthority in XAUTHORITY_FILE char *src; char *dest = RUN_XAUTHORITY_FILE; - // create an empty file + // create an empty file as root, and change ownership to user FILE *fp = fopen(dest, "w"); if (fp) { fprintf(fp, "\n"); @@ -124,27 +123,8 @@ static int store_xauthority(void) { return 0; } - pid_t child = fork(); - if (child < 0) - errExit("fork"); - if (child == 0) { - // drop privileges - drop_privs(0); - - // copy, set permissions and ownership - int rv = copy_file(src, dest, getuid(), getgid(), 0600); - if (rv) - fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n"); - else { - fs_logger2("clone", dest); - } -#ifdef HAVE_GCOV - __gcov_flush(); -#endif - _exit(0); - } - // wait for the child to finish - waitpid(child, NULL, 0); + copy_file_as_user(src, dest, getuid(), getgid(), 0600); + fs_logger2("clone", dest); return 1; // file copied } @@ -152,9 +132,10 @@ static int store_xauthority(void) { } static int store_asoundrc(void) { + // put a copy of .Xauthority in XAUTHORITY_FILE char *src; char *dest = RUN_ASOUNDRC_FILE; - // create an empty file + // create an empty file as root, and change ownership to user FILE *fp = fopen(dest, "w"); if (fp) { fprintf(fp, "\n"); @@ -182,27 +163,8 @@ static int store_asoundrc(void) { free(rp); } - pid_t child = fork(); - if (child < 0) - errExit("fork"); - if (child == 0) { - // drop privileges - drop_privs(0); - - // copy, set permissions and ownership - int rv = copy_file(src, dest, getuid(), getgid(), 0644); - if (rv) - fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n"); - else { - fs_logger2("clone", dest); - } -#ifdef HAVE_GCOV - __gcov_flush(); -#endif - _exit(0); - } - // wait for the child to finish - waitpid(child, NULL, 0); + copy_file_as_user(src, dest, getuid(), getgid(), 0644); + fs_logger2("clone", dest); return 1; // file copied } @@ -222,27 +184,8 @@ static void copy_xauthority(void) { exit(1); } - pid_t child = fork(); - if (child < 0) - errExit("fork"); - if (child == 0) { - // drop privileges - drop_privs(0); - - // copy, set permissions and ownership - int rv = copy_file(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR); - if (rv) - fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n"); - else { - fs_logger2("clone", dest); - } -#ifdef HAVE_GCOV - __gcov_flush(); -#endif - _exit(0); - } - // wait for the child to finish - waitpid(child, NULL, 0); + copy_file_as_user(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR); + fs_logger2("clone", dest); // delete the temporary file unlink(src); @@ -261,27 +204,8 @@ static void copy_asoundrc(void) { exit(1); } - pid_t child = fork(); - if (child < 0) - errExit("fork"); - if (child == 0) { - // drop privileges - drop_privs(0); - - // copy, set permissions and ownership - int rv = copy_file(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR); - if (rv) - fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n"); - else { - fs_logger2("clone", dest); - } -#ifdef HAVE_GCOV - __gcov_flush(); -#endif - _exit(0); - } - // wait for the child to finish - waitpid(child, NULL, 0); + copy_file_as_user(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR); + fs_logger2("clone", dest); // delete the temporary file unlink(src); diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index 14a7f03dd..f0f95a80e 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -22,6 +22,7 @@ #include #include #include +#include static void disable_file(const char *path, const char *file) { assert(file); diff --git a/src/firejail/util.c b/src/firejail/util.c index 5b94aa288..2d3563093 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -28,6 +28,7 @@ #include #include #include +#include #define MAX_GROUPS 1024 // drop privileges @@ -218,6 +219,51 @@ int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, m return 0; } +// return -1 if error, 0 if no error +void copy_file_as_user(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode) { + pid_t child = fork(); + if (child < 0) + errExit("fork"); + if (child == 0) { + // drop privileges + drop_privs(0); + + // copy, set permissions and ownership + int rv = copy_file(srcname, destname, uid, gid, mode); + if (rv) + fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n"); +#ifdef HAVE_GCOV + __gcov_flush(); +#endif + _exit(0); + } + // wait for the child to finish + waitpid(child, NULL, 0); +} + +// return -1 if error, 0 if no error +void touch_file_as_user(const char *fname, uid_t uid, gid_t gid, mode_t mode) { + pid_t child = fork(); + if (child < 0) + errExit("fork"); + if (child == 0) { + // drop privileges + drop_privs(0); + + FILE *fp = fopen(fname, "w"); + if (fp) { + fprintf(fp, "\n"); + SET_PERMS_STREAM(fp, uid, gid, mode); + fclose(fp); + } +#ifdef HAVE_GCOV + __gcov_flush(); +#endif + _exit(0); + } + // wait for the child to finish + waitpid(child, NULL, 0); +} // return 1 if the file is a directory int is_dir(const char *fname) { -- cgit v1.2.3-54-g00ecf