private-bin: fix #4626, refactor symlink detection

src/firejail/firejail.h

@@ -709,6 +709,8 @@ void pulseaudio_disable(void);
 void fs_private_bin_list(void);
 
 // fs_lib.c
+int is_firejail_link(const char *fname);
+char *find_in_path(const char *program);
 void fs_private_lib(void);
 
 // protocol.c

src/firejail/fs_bin.c

@@ -43,7 +43,6 @@ static char *paths[] = {
 static char *check_dir_or_file(const char *name) {
 	assert(name);
 	struct stat s;
-	char *fname = NULL;
 
 	int i = 0;
 	while (paths[i]) {
@@ -54,45 +53,28 @@ static char *check_dir_or_file(const char *name) {
 		}
 
 		// check file
+		char *fname;
 		if (asprintf(&fname, "%s/%s", paths[i], name) == -1)
 			errExit("asprintf");
 		if (arg_debug)
 			printf("Checking %s/%s\n", paths[i], name);
-		if (stat(fname, &s) == 0 && !S_ISDIR(s.st_mode)) { // do not allow directories
-			// check symlink to firejail executable in /usr/local/bin
-			if (strcmp(paths[i], "/usr/local/bin") == 0 && is_link(fname)) {
-				/* coverity[toctou] */
-				char *actual_path = realpath(fname, NULL);
-				if (actual_path) {
-					char *ptr = strstr(actual_path, "/firejail");
-					if (ptr && strlen(ptr) == strlen("/firejail")) {
-						if (arg_debug)
-							printf("firejail exec symlink detected\n");
-						free(actual_path);
-						free(fname);
-						fname = NULL;
-						i++;
-						continue;
-					}
-					free(actual_path);
-				}
-
-			}
-			break; // file found
+		if (stat(fname, &s) == 0 &&
+		    !S_ISDIR(s.st_mode) &&	// do not allow directories
+		    !is_firejail_link(fname)) {	// skip symlinks to firejail executable, as created by firecfg
+				free(fname);
+				break; // file found
 		}
 
 		free(fname);
-		fname = NULL;
 		i++;
 	}
 
-	if (!fname) {
+	if (!paths[i]) {
 		if (arg_debug)
 			fwarning("file %s not found\n", name);
 		return NULL;
 	}
 
-	free(fname);
 	return paths[i];
 }
 
@@ -256,6 +238,9 @@ static void globbing(char *fname) {
 			// testing for GLOB_NOCHECK - no pattern matched returns the original pattern
 			if (strcmp(globbuf.gl_pathv[j], pattern) == 0)
 				continue;
+			// skip symlinks to firejail executable, as created by firecfg
+			if (is_firejail_link(globbuf.gl_pathv[j]))
+				continue;
 
 			duplicate(globbuf.gl_pathv[j]);
 		}

src/firejail/fs_lib.c

@@ -61,17 +61,32 @@ static int valid_full_path(const char *full_path) {
 	return 0;
 }
 
+// return 1 if symlink to firejail executable
+int is_firejail_link(const char *fname) {
+	EUID_ASSERT();
+
+	if (!is_link(fname))
+		return 0;
+
+	// char *rp = realpath_as_user(fname, NULL);
+	char *rp = realpath(fname, NULL);
+	if (!rp)
+		return 0;
+
+	int rv = 0;
+	const char *base = gnu_basename(rp);
+	if (strcmp(base, "firejail") == 0)
+		rv = 1;
+
+	free(rp);
+	return rv;
+}
+
 char *find_in_path(const char *program) {
 	EUID_ASSERT();
 	if (arg_debug)
 		printf("Searching $PATH for %s\n", program);
 
-	char self[MAXBUF];
-	ssize_t len = readlink("/proc/self/exe", self, MAXBUF - 1);
-	if (len < 0)
-		errExit("readlink");
-	self[len] = '\0';
-
 	const char *path = env_get("PATH");
 	if (!path)
 		return NULL;
@@ -88,18 +103,12 @@ char *find_in_path(const char *program) {
 		if (arg_debug)
 			printf("trying #%s#\n", fname);
 		struct stat s;
-		if (stat(fname, &s) == 0) {
-			// but skip links created by firecfg
-			char *rp = realpath(fname, NULL);
-			if (!rp)
-				errExit("realpath");
-			if (strcmp(self, rp) != 0) {
-				free(rp);
-				free(dup);
-				return fname;
-			}
-			free(rp);
+		if (stat(fname, &s) == 0 &&
+		    !is_firejail_link(fname)) { // skip links created by firecfg
+			free(dup);
+			return fname;
 		}
+
 		free(fname);
 		tok = strtok(NULL, ":");
 	}

src/firejail/run_symlink.c

@@ -22,7 +22,6 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
-extern char *find_in_path(const char *program);
 
 void run_symlink(int argc, char **argv, int run_as_is) {
 	EUID_ASSERT();