From f8762bcff754911bc4a2a2c8d998f5ba93f4a384 Mon Sep 17 00:00:00 2001
From: smitsohu <smitsohu@gmail.com>
Date: Sat, 11 Aug 2018 23:32:40 +0200
Subject: various small improvements, fixes, nitpicks

---
 src/firejail/fs_whitelist.c | 8 +++-----
 src/firejail/join.c         | 2 +-
 src/firejail/ls.c           | 5 +++++
 src/firejail/main.c         | 4 ++++
 src/firejail/run_symlink.c  | 1 +
 src/firejail/util.c         | 3 +--
 src/firejail/x11.c          | 5 -----
 src/fnetfilter/main.c       | 6 +++++-
 8 files changed, 20 insertions(+), 14 deletions(-)

(limited to 'src')

diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c
index bfcf9c209..c3d34e259 100644
--- a/src/firejail/fs_whitelist.c
+++ b/src/firejail/fs_whitelist.c
@@ -65,8 +65,7 @@ static int mkpath(const char* path, mode_t mode) {
 
 	// don't create the last path element
 	char *p = strrchr(dup, '/');
-	if (!p)
-		errExit("strrchr");
+	assert(p);
 	*p = '\0';
 
 	int parentfd = open("/", O_PATH|O_DIRECTORY|O_CLOEXEC);
@@ -77,8 +76,7 @@ static int mkpath(const char* path, mode_t mode) {
 	int done = 0;
 	int fd = -1;
 	char *tok = strtok(dup, "/");
-	if (!tok)
-		errExit("strtok");
+	assert(tok); // path is no top level directory
 	while (tok) {
 		// skip all instances of "/./"
 		if (strcmp(tok, ".") == 0) {
@@ -398,7 +396,7 @@ void fs_whitelist(void) {
 		assert(new_name);
 
 		// trim trailing slashes or dots
-		char *end = strrchr(new_name, '\0');
+		char *end = strchr(new_name, '\0');
 		assert(end);
 		if ((end - new_name) > 1) {
 			end--;
diff --git a/src/firejail/join.c b/src/firejail/join.c
index a75262711..729c7f797 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -214,7 +214,7 @@ static void extract_umask(pid_t pid) {
 	free(fname);
 	if (!fp)
 		return;
-	if (fscanf(fp, "%4o", &orig_umask) < 1) {
+	if (fscanf(fp, "%3o", &orig_umask) < 1) {
 		fprintf(stderr, "Error: cannot read umask\n");
 		exit(1);
 	}
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index 79e4b679b..601cab4f8 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -198,6 +198,10 @@ char *expand_path(const char *path) {
 	}
 	else {
 		// assume the file is in current working directory
+		if (!cfg.cwd) {
+			fprintf(stderr, "Error: current working directory has been deleted\n");
+			exit(1);
+		}
 		if (asprintf(&fname, "%s/%s", cfg.cwd, path) == -1)
 			errExit("asprintf");
 	}
@@ -206,6 +210,7 @@ char *expand_path(const char *path) {
 
 void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
 	EUID_ASSERT();
+	assert(path1);
 
 	// if the pid is that of a firejail  process, use the pid of the first child process
 	EUID_ROOT();
diff --git a/src/firejail/main.c b/src/firejail/main.c
index 0651e2f0a..b064155f4 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -241,7 +241,10 @@ static void init_cfg(int argc, char **argv) {
 		fprintf(stderr, "Error: user %s doesn't have a user directory assigned\n", cfg.username);
 		exit(1);
 	}
+
 	cfg.cwd = getcwd(NULL, 0);
+	if (!cfg.cwd && errno != ENOENT)
+		errExit("getcwd");
 
 	// check user database
 	if (!firejail_user_check(cfg.username)) {
@@ -830,6 +833,7 @@ static void run_builder(int argc, char **argv) {
 	(void) argc;
 
 	// drop privileges
+	EUID_ROOT();
 	if (setgid(getgid()) < 0)
 		errExit("setgid/getgid");
 	if (setuid(getuid()) < 0)
diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c
index 5714206d4..ec8e0f1e5 100644
--- a/src/firejail/run_symlink.c
+++ b/src/firejail/run_symlink.c
@@ -34,6 +34,7 @@ void run_symlink(int argc, char **argv, int run_as_is) {
 		return;
 
 	// drop privileges
+	EUID_ROOT();
 	if (setgid(getgid()) < 0)
 		errExit("setgid/getgid");
 	if (setuid(getuid()) < 0)
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 67776b36c..329ae141b 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -1006,8 +1006,7 @@ int safe_fd(const char *path, int flags) {
 		errExit("strdup");
 
 	char *p = strrchr(dup, '/');
-	if (p == NULL)
-		errExit("strrchr");
+	assert(p);
 	// reject trailing slash, root directory
 	if (*(p + 1) == '\0')
 		goto errexit;
diff --git a/src/firejail/x11.c b/src/firejail/x11.c
index 9cbe6598e..e40ca0f05 100644
--- a/src/firejail/x11.c
+++ b/src/firejail/x11.c
@@ -30,12 +30,7 @@
 #include <sys/wait.h>
 #include <errno.h>
 #include <limits.h>
-
-// on Debian 7 we are missing O_PATH definition
 #include <fcntl.h>
-#ifndef O_PATH
-#define O_PATH		010000000
-#endif
 
 
 // Parse the DISPLAY environment variable and return a display number.
diff --git a/src/fnetfilter/main.c b/src/fnetfilter/main.c
index d8b950e8f..34ebf5926 100644
--- a/src/fnetfilter/main.c
+++ b/src/fnetfilter/main.c
@@ -79,13 +79,17 @@ static void process_template(char *src, const char *dest) {
 	*arg_start = '\0';
 	arg_start++;
 	if (*arg_start == '\0') {
-		fprintf(stderr, "Error fnetfilter: you need to provide at least on argument\n");
+		fprintf(stderr, "Error fnetfilter: you need to provide at least one argument\n");
 		exit(1);
 	}
 
 	// extract the arguments from command line
 	char *token = strtok(arg_start, ",");
 	while (token) {
+		if (argcnt == MAXARGS) {
+			fprintf(stderr, "Error fnetfilter: only up to %u arguments are supported\n", (unsigned) MAXARGS);
+			exit(1);
+		}
 		// look for abnormal things
 		int len = strlen(token);
 		if (strcspn(token, "\\&!?\"'<>%^(){};,*[]") != (size_t)len) {
-- 
cgit v1.2.3-54-g00ecf