From 7f0b5ddd8881c5276138b061109ab48eb5165201 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Fri, 22 Oct 2021 22:00:16 +0200 Subject: private-bin: fix #4626, refactor symlink detection --- src/firejail/firejail.h | 2 ++ src/firejail/fs_bin.c | 35 ++++++++++------------------------- src/firejail/fs_lib.c | 43 ++++++++++++++++++++++++++----------------- src/firejail/run_symlink.c | 1 - 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 #include -extern char *find_in_path(const char *program); void run_symlink(int argc, char **argv, int run_as_is) { EUID_ASSERT(); -- cgit v1.2.3-54-g00ecf