private-bin: fix #4626, refactor symlink detection

4 files changed, 38 insertions, 43 deletions

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index a6924b830..ec789cd63 100644
--- a/src/firejail/firejail.h
+++ b/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
diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c
index 61398f12b..d485de05a 100644
--- a/src/firejail/fs_bin.c
+++ b/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]);
                 }
diff --git a/src/firejail/fs_lib.c b/src/firejail/fs_lib.c
index 848c186fa..249b41a12 100644
--- a/src/firejail/fs_lib.c
+++ b/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, ":");
         }
diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c
index 77fac5438..6397418d1 100644
--- a/src/firejail/run_symlink.c
+++ b/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();