From d6ed2e783a32385e94b8cbbe1673498ab4aa1ebb Mon Sep 17 00:00:00 2001
From: smitsohu <smitsohu@gmail.com>
Date: Sat, 19 Jan 2019 22:40:13 +0100
Subject: improve chroot error handling

---
 src/firejail/fs.c | 81 ++++++++++++++++++++-----------------------------------
 1 file changed, 29 insertions(+), 52 deletions(-)

diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index 5edcdd58f..ce2b1a8bc 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -30,6 +30,7 @@
 #include <fcntl.h>
 
 #define MAX_BUF 4096
+#define EMPTY_STRING ("")
 // check noblacklist statements not matched by a proper blacklist in disable-*.inc files
 //#define TEST_NO_BLACKLIST_MATCHING
 
@@ -1215,9 +1216,8 @@ void fs_overlayfs(void) {
 void fs_check_chroot_dir(const char *rootdir) {
 	EUID_ASSERT();
 	assert(rootdir);
+	char *dir = EMPTY_STRING;
 	struct stat s;
-	int fd = -1;
-	int parentfd = -1;
 
 	char *overlay;
 	if (asprintf(&overlay, "%s/.firejail", cfg.homedir) == -1)
@@ -1229,7 +1229,7 @@ void fs_check_chroot_dir(const char *rootdir) {
 	free(overlay);
 
 	// fails if there is any symlink or if rootdir is not a directory
-	parentfd = safe_fd(rootdir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+	int parentfd = safe_fd(rootdir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
 	if (parentfd == -1) {
 		fprintf(stderr, "Error: invalid chroot directory %s\n", rootdir);
 		exit(1);
@@ -1248,78 +1248,58 @@ void fs_check_chroot_dir(const char *rootdir) {
 	}
 
 	// check /dev
-	char *dir = "dev";
-	fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-	if (fd == -1) {
-		if (errno == ENOENT)
-			goto error1;
-		else
-			goto error2;
-	}
+	dir = "dev";
+	int fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
+	if (fd == -1)
+		goto error1;
 	if (fstat(fd, &s) == -1)
 		errExit("fstat");
 	if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-		goto error3;
+		goto error2;
 	close(fd);
 
 	// check /var/tmp
 	dir = "var/tmp";
 	fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-	if (fd == -1) {
-		if (errno == ENOENT)
-			goto error1;
-		else
-			goto error2;
-	}
+	if (fd == -1)
+		goto error1;
 	if (fstat(fd, &s) == -1)
 		errExit("fstat");
 	if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-		goto error3;
+		goto error2;
 	close(fd);
 
 	// check /proc
 	dir = "proc";
 	fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-	if (fd == -1) {
-		if (errno == ENOENT)
-			goto error1;
-		else
-			goto error2;
-	}
+	if (fd == -1)
+		goto error1;
 	if (fstat(fd, &s) == -1)
 		errExit("fstat");
 	if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-		goto error3;
+		goto error2;
 	close(fd);
 
 	// check /tmp
 	dir = "tmp";
 	fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-	if (fd == -1) {
-		if (errno == ENOENT)
-			goto error1;
-		else
-			goto error2;
-	}
+	if (fd == -1)
+		goto error1;
 	if (fstat(fd, &s) == -1)
 		errExit("fstat");
 	if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-		goto error3;
+		goto error2;
 	close(fd);
 
 	// check /etc
 	dir = "etc";
 	fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-	if (fd == -1) {
-		if (errno == ENOENT)
-			goto error1;
-		else
-			goto error2;
-	}
+	if (fd == -1)
+		goto error1;
 	if (fstat(fd, &s) == -1)
 		errExit("fstat");
 	if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-		goto error3;
+		goto error2;
 	if (((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
 		fprintf(stderr, "Error: only root user should be given write permission on chroot /etc\n");
 		exit(1);
@@ -1358,16 +1338,12 @@ void fs_check_chroot_dir(const char *rootdir) {
 	if (getenv("FIREJAIL_X11")) {
 		dir = "tmp/.X11-unix";
 		fd = openat(parentfd, dir, O_PATH|O_CLOEXEC);
-		if (fd == -1) {
-			if (errno == ENOENT)
-				goto error1;
-			else
-				goto error2;
-		}
+		if (fd == -1)
+			goto error1;
 		if (fstat(fd, &s) == -1)
 			errExit("fstat");
 		if (!S_ISDIR(s.st_mode) || s.st_uid != 0)
-			goto error3;
+			goto error2;
 		close(fd);
 	}
 
@@ -1375,13 +1351,14 @@ void fs_check_chroot_dir(const char *rootdir) {
 	return;
 
 error1:
-	fprintf(stderr, "Error: cannot find /%s in chroot directory\n", dir);
+	if (errno == ENOENT)
+		fprintf(stderr, "Error: cannot find /%s in chroot directory\n", dir);
+	else {
+		perror("open");
+		fprintf(stderr, "Error: cannot open /%s in chroot directory\n", dir);
+	}
 	exit(1);
 error2:
-	perror("open");
-	fprintf(stderr, "Error: cannot open /%s in chroot directory\n", dir);
-	exit(1);
-error3:
 	fprintf(stderr, "Error: chroot /%s should be a directory owned by root\n", dir);
 	exit(1);
 }
-- 
cgit v1.2.3-54-g00ecf