Skip to content

Commit

Permalink
Merge pull request #742 from manevich/security
Browse files Browse the repository at this point in the history
Tighten security
  • Loading branch information
netblue30 authored Aug 24, 2016
2 parents 1bb4451 + c321020 commit 75e48b2
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 122 deletions.
24 changes: 14 additions & 10 deletions src/firejail/appimage.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,20 @@ void appimage_set(const char *appimage_path) {
assert(appimage_path);
assert(devloop == NULL); // don't call this twice!
EUID_ASSERT();

// check appimage_path
if (access(appimage_path, R_OK) == -1) {
fprintf(stderr, "Error: cannot access AppImage file\n");
exit(1);
}


// open as user to prevent race condition
int ffd = open(appimage_path, O_RDONLY|O_CLOEXEC);
if (ffd == -1)
errExit("open");

EUID_ROOT();

// find or allocate a free loop device to use
int cfd = open("/dev/loop-control", O_RDWR);
int devnr = ioctl(cfd, LOOP_CTL_GET_FREE);
Expand All @@ -59,7 +64,6 @@ void appimage_set(const char *appimage_path) {
if (asprintf(&devloop, "/dev/loop%d", devnr) == -1)
errExit("asprintf");

int ffd = open(appimage_path, O_RDONLY|O_CLOEXEC);
int lfd = open(devloop, O_RDONLY);
if (ioctl(lfd, LOOP_SET_FD, ffd) == -1) {
fprintf(stderr, "Error: cannot configure the loopback device\n");
Expand All @@ -68,22 +72,22 @@ void appimage_set(const char *appimage_path) {
close(lfd);
close(ffd);

EUID_USER();

// creates directory with perms 0700
char dirname[] = "/tmp/firejail-mnt-XXXXXX";
mntdir = strdup(mkdtemp(dirname));
if (mntdir == NULL) {
fprintf(stderr, "Error: cannot create temporary directory\n");
exit(1);
}
mkdir(mntdir, 755);
if (chown(mntdir, getuid(), getgid()) == -1)
errExit("chown");
if (chmod(mntdir, 755) == -1)
errExit("chmod");
ASSERT_PERMS(mntdir, getuid(), getgid(), 0700);

char *mode;
if (asprintf(&mode, "mode=755,uid=%d,gid=%d", getuid(), getgid()) == -1)
if (asprintf(&mode, "mode=700,uid=%d,gid=%d", getuid(), getgid()) == -1)
errExit("asprintf");

EUID_ROOT();
if (mount(devloop, mntdir, "iso9660",MS_MGC_VAL|MS_RDONLY, mode) < 0)
errExit("mounting appimage");

Expand Down
23 changes: 22 additions & 1 deletion src/firejail/firejail.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,27 @@
#define DEFAULT_ROOT_PROFILE "server"
#define MAX_INCLUDE_LEVEL 6 // include levels in profile files


#define ASSERT_PERMS(file, uid, gid, mode) \
do { \
assert(file);\
struct stat s;\
if (stat(file, &s) == -1) errExit("stat");\
assert(s.st_uid == uid && s.st_gid == gid && (s.st_mode & 07777) == mode);\
} while (0)
#define ASSERT_PERMS_FD(fd, uid, gid, mode) \
do { \
struct stat s;\
if (stat(fd, &s) == -1) errExit("stat");\
assert(s.st_uid == uid && s.st_gid == gid && (s.st_mode & 07777) == mode);\
} while (0)
#define ASSERT_PERMS_STREAM(file, uid, gid, mode) \
do { \
int fd = fileno(file);\
if (fd == -1) errExit("fileno");\
ASSERT_PERMS_FD(fd, uid, gid, mode);\
} while (0)

// main.c
typedef struct bridge_t {
// on the host
Expand Down Expand Up @@ -386,7 +407,7 @@ void logsignal(int s);
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);
int copy_file(const char *srcname, const char *destname, 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);
Expand Down
120 changes: 34 additions & 86 deletions src/firejail/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,25 @@

static void fs_rdwr(const char *dir);

static void create_dir_as_root(const char *dir, mode_t mode) {
assert(dir);
if (arg_debug)
printf("Creating %s directory\n", dir);

if (mkdir(dir, mode) == -1)
errExit("mkdir");

ASSERT_PERMS(dir, 0, 0, mode);
}

static void create_empty_dir(void) {
struct stat s;

if (stat(RUN_RO_DIR, &s)) {
/* coverity[toctou] */
int rv = mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR);
if (rv == -1)
errExit("mkdir");
if (chown(RUN_RO_DIR, 0, 0) < 0)
errExit("chown");
if (mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR) == -1)
errExit("mkdir");
ASSERT_PERMS(RUN_RO_DIR, 0, 0, S_IRUSR | S_IXUSR);
}
}

Expand All @@ -50,11 +59,16 @@ static void create_empty_file(void) {
FILE *fp = fopen(RUN_RO_FILE, "w");
if (!fp)
errExit("fopen");
fclose(fp);
if (chown(RUN_RO_FILE, 0, 0) < 0)

int fd = fileno(fp);
if (fd == -1)
errExit("fileno");
if (fchown(fd, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_RO_FILE, S_IRUSR) < 0)
if (fchmod(fd, S_IRUSR) < 0)
errExit("chown");

fclose(fp);
}
}

Expand All @@ -64,16 +78,7 @@ void fs_build_firejail_dir(void) {

// CentOS 6 doesn't have /run directory
if (stat(RUN_FIREJAIL_BASEDIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_BASEDIR);
/* coverity[toctou] */
int rv = mkdir(RUN_FIREJAIL_BASEDIR, 0755);
if (rv == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_BASEDIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_BASEDIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_BASEDIR, 0755);
}
else { // check /tmp/firejail directory belongs to root end exit if doesn't!
if (s.st_uid != 0 || s.st_gid != 0) {
Expand All @@ -83,61 +88,23 @@ void fs_build_firejail_dir(void) {
}

if (stat(RUN_FIREJAIL_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_DIR);
/* coverity[toctou] */
int rv = mkdir(RUN_FIREJAIL_DIR, 0755);
if (rv == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_DIR, 0755);
}

if (stat(RUN_FIREJAIL_NETWORK_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_NETWORK_DIR);

if (mkdir(RUN_FIREJAIL_NETWORK_DIR, 0755) == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_NETWORK_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_NETWORK_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_NETWORK_DIR, 0755);
}

if (stat(RUN_FIREJAIL_BANDWIDTH_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_BANDWIDTH_DIR);
if (mkdir(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_BANDWIDTH_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_BANDWIDTH_DIR, 0755);
}

if (stat(RUN_FIREJAIL_NAME_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_NAME_DIR);
if (mkdir(RUN_FIREJAIL_NAME_DIR, 0755) == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_NAME_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_NAME_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_NAME_DIR, 0755);
}

if (stat(RUN_FIREJAIL_X11_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_FIREJAIL_X11_DIR);
if (mkdir(RUN_FIREJAIL_X11_DIR, 0755) == -1)
errExit("mkdir");
if (chown(RUN_FIREJAIL_X11_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_FIREJAIL_X11_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_FIREJAIL_X11_DIR, 0755);
}

create_empty_dir();
Expand All @@ -160,16 +127,7 @@ void fs_build_mnt_dir(void) {

// create /run/firejail/mnt directory
if (stat(RUN_MNT_DIR, &s)) {
if (arg_debug)
printf("Creating %s directory\n", RUN_MNT_DIR);
/* coverity[toctou] */
int rv = mkdir(RUN_MNT_DIR, 0755);
if (rv == -1)
errExit("mkdir");
if (chown(RUN_MNT_DIR, 0, 0) < 0)
errExit("chown");
if (chmod(RUN_MNT_DIR, 0755) < 0)
errExit("chmod");
create_dir_as_root(RUN_MNT_DIR, 0755);
}

// ... and mount tmpfs on top of it
Expand Down Expand Up @@ -202,16 +160,12 @@ void fs_build_cp_command(void) {
fprintf(stderr, "Error: invalid /bin/cp file\n");
exit(1);
}
int rv = copy_file(fname, RUN_CP_COMMAND);
int rv = copy_file(fname, RUN_CP_COMMAND, 0, 0, 0755);
if (rv) {
fprintf(stderr, "Error: cannot access /bin/cp\n");
exit(1);
}
/* coverity[toctou] */
if (chown(RUN_CP_COMMAND, 0, 0))
errExit("chown");
if (chmod(RUN_CP_COMMAND, 0755))
errExit("chmod");
ASSERT_PERMS(RUN_CP_COMMAND, 0, 0, 0755);

free(fname);
}
Expand Down Expand Up @@ -827,10 +781,7 @@ char *fs_check_overlay_dir(const char *subdirname, int allow_reuse) {
/* coverity[toctou] */
if (mkdir(dirname, 0700))
errExit("mkdir");
if (chown(dirname, getuid(), getgid()) < 0)
errExit("chown");
if (chmod(dirname, 0700) < 0)
errExit("chmod");
ASSERT_PERMS(dirname, getuid(), getgid(), 0700);
}
else if (is_link(dirname)) {
fprintf(stderr, "Error: invalid ~/.firejail directory\n");
Expand Down Expand Up @@ -917,10 +868,7 @@ void fs_overlayfs(void) {
errExit("asprintf");
if (mkdir(oroot, 0755))
errExit("mkdir");
if (chown(oroot, 0, 0) < 0)
errExit("chown");
if (chmod(oroot, 0755) < 0)
errExit("chmod");
ASSERT_PERMS(oroot, 0, 0, 0755);

struct stat s;
char *basedir = RUN_MNT_DIR;
Expand Down Expand Up @@ -1259,7 +1207,7 @@ void fs_chroot(const char *rootdir) {
fprintf(stderr, "Error: invalid %s file\n", fname);
exit(1);
}
if (copy_file("/etc/resolv.conf", fname) == -1)
if (copy_file("/etc/resolv.conf", fname, 0, 0, 0644) == -1)
fprintf(stderr, "Warning: /etc/resolv.conf not initialized\n");
}

Expand Down
25 changes: 8 additions & 17 deletions src/firejail/fs_home.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
if (stat(fname, &s) == 0)
return;
if (stat("/etc/skel/.zshrc", &s) == 0) {
if (copy_file("/etc/skel/.zshrc", fname) == 0) {
if (chown(fname, u, g) == -1)
errExit("chown");
if (copy_file("/etc/skel/.zshrc", fname, u, g, 0644) == 0) {
fs_logger("clone /etc/skel/.zshrc");
}
}
Expand Down Expand Up @@ -73,9 +71,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
if (stat(fname, &s) == 0)
return;
if (stat("/etc/skel/.cshrc", &s) == 0) {
if (copy_file("/etc/skel/.cshrc", fname) == 0) {
if (chown(fname, u, g) == -1)
errExit("chown");
if (copy_file("/etc/skel/.cshrc", fname, u, g, 0644) == 0) {
fs_logger("clone /etc/skel/.cshrc");
}
}
Expand Down Expand Up @@ -104,10 +100,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
if (stat(fname, &s) == 0)
return;
if (stat("/etc/skel/.bashrc", &s) == 0) {
if (copy_file("/etc/skel/.bashrc", fname) == 0) {
/* coverity[toctou] */
if (chown(fname, u, g) == -1)
errExit("chown");
if (copy_file("/etc/skel/.bashrc", fname, u, g, 0644) == 0) {
fs_logger("clone /etc/skel/.bashrc");
}
}
Expand All @@ -131,7 +124,7 @@ static int store_xauthority(void) {
exit(1);
}

int rv = copy_file(src, dest);
int rv = copy_file(src, dest, -1, -1, 0600);
if (rv) {
fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n");
return 0;
Expand Down Expand Up @@ -167,7 +160,7 @@ static int store_asoundrc(void) {
free(rp);
}

int rv = copy_file(src, dest);
int rv = copy_file(src, dest, -1, -1, -0644);
if (rv) {
fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n");
return 0;
Expand All @@ -184,7 +177,7 @@ static void copy_xauthority(void) {
char *dest;
if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1)
errExit("asprintf");
int rv = copy_file(src, dest);
int rv = copy_file(src, dest, -1, -1, 0600);
if (rv)
fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n");
else {
Expand All @@ -207,7 +200,7 @@ static void copy_asoundrc(void) {
char *dest;
if (asprintf(&dest, "%s/.asoundrc", cfg.homedir) == -1)
errExit("asprintf");
int rv = copy_file(src, dest);
int rv = copy_file(src, dest, -1 , -1, 0644);
if (rv)
fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n");
else {
Expand Down Expand Up @@ -360,11 +353,9 @@ int fs_copydir(const char *path, const struct stat *st, int ftype, struct FTW *s
return(0);
if (stat(path, &s) == 0) {
if(ftype == FTW_F) {
if (copy_file(path, dest) == 0) {
if (copy_file(path, dest, u, g, 0644) == 0) {
if (arg_debug)
printf("copy from %s to %s\n", path, dest);
if (chown(dest, u, g) == -1)
errExit("chown");
fs_logger2("clone", path);
}
}
Expand Down
6 changes: 1 addition & 5 deletions src/firejail/ls.c
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,7 @@ void sandboxfs(int op, pid_t pid, const char *path) {
}
// copy file
EUID_ROOT();
copy_file(src_fname, dest_fname);
if (chown(dest_fname, getuid(), getgid()) == -1)
errExit("chown");
if (chmod(dest_fname, 0644) == -1)
errExit("chmod");
copy_file(src_fname, dest_fname, getuid(), getgid(), 0644);
printf("Transfer complete\n");
EUID_USER();
}
Expand Down
Loading

0 comments on commit 75e48b2

Please sign in to comment.