diff options
author | smitsohu <smitsohu@gmail.com> | 2018-09-05 00:02:22 +0200 |
---|---|---|
committer | smitsohu <smitsohu@gmail.com> | 2018-09-05 00:02:22 +0200 |
commit | 884d59f5a53cd51d25ed6ee6d4933081321e770c (patch) | |
tree | d2440c6899f16e919b52557206bbdbff417565ce /src/firejail/util.c | |
parent | fix --shell (diff) | |
download | firejail-884d59f5a53cd51d25ed6ee6d4933081321e770c.tar.gz firejail-884d59f5a53cd51d25ed6ee6d4933081321e770c.tar.zst firejail-884d59f5a53cd51d25ed6ee6d4933081321e770c.zip |
improve safe_fd() function for better readability and auditability
Diffstat (limited to 'src/firejail/util.c')
-rw-r--r-- | src/firejail/util.c | 57 |
1 files changed, 30 insertions, 27 deletions
diff --git a/src/firejail/util.c b/src/firejail/util.c index ff2bceacd..f677b44eb 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c | |||
@@ -1049,7 +1049,7 @@ void disable_file_path(const char *path, const char *file) { | |||
1049 | } | 1049 | } |
1050 | 1050 | ||
1051 | // The returned file descriptor should be suitable for privileged operations on | 1051 | // The returned file descriptor should be suitable for privileged operations on |
1052 | // user controlled paths. Passed flags are ignored if path is a top level directory. | 1052 | // user controlled paths |
1053 | int safe_fd(const char *path, int flags) { | 1053 | int safe_fd(const char *path, int flags) { |
1054 | assert(path); | 1054 | assert(path); |
1055 | 1055 | ||
@@ -1059,13 +1059,7 @@ int safe_fd(const char *path, int flags) { | |||
1059 | // reject ".." | 1059 | // reject ".." |
1060 | if (strstr(path, "..")) | 1060 | if (strstr(path, "..")) |
1061 | goto errexit; | 1061 | goto errexit; |
1062 | 1062 | char *p = strrchr(path, '/'); | |
1063 | // work with a copy of path | ||
1064 | char *dup = strdup(path); | ||
1065 | if (dup == NULL) | ||
1066 | errExit("strdup"); | ||
1067 | |||
1068 | char *p = strrchr(dup, '/'); | ||
1069 | assert(p); | 1063 | assert(p); |
1070 | // reject trailing slash, root directory | 1064 | // reject trailing slash, root directory |
1071 | if (*(p + 1) == '\0') | 1065 | if (*(p + 1) == '\0') |
@@ -1073,45 +1067,54 @@ int safe_fd(const char *path, int flags) { | |||
1073 | // reject trailing dot | 1067 | // reject trailing dot |
1074 | if (*(p + 1) == '.' && *(p + 2) == '\0') | 1068 | if (*(p + 1) == '.' && *(p + 2) == '\0') |
1075 | goto errexit; | 1069 | goto errexit; |
1076 | // if there is more than one path segment, keep the last one for later | 1070 | |
1077 | if (p != dup) | 1071 | // work with a copy of path |
1078 | *p = '\0'; | 1072 | char *dup = strdup(path); |
1073 | if (!dup) | ||
1074 | errExit("strdup"); | ||
1079 | 1075 | ||
1080 | int parentfd = open("/", O_PATH|O_DIRECTORY|O_CLOEXEC); | 1076 | int parentfd = open("/", O_PATH|O_DIRECTORY|O_CLOEXEC); |
1081 | if (parentfd == -1) | 1077 | if (parentfd == -1) |
1082 | errExit("open"); | 1078 | errExit("open"); |
1083 | 1079 | ||
1084 | // traverse the path and return -1 if a symlink is encountered | 1080 | // traverse the path and return -1 if a symlink is encountered |
1085 | int weird_pathname = 1; | ||
1086 | int fd = -1; | 1081 | int fd = -1; |
1082 | char *current_tok = NULL; | ||
1087 | char *tok = strtok(dup, "/"); | 1083 | char *tok = strtok(dup, "/"); |
1084 | assert(tok); | ||
1088 | while (tok) { | 1085 | while (tok) { |
1089 | // skip all "/./" | 1086 | // skip all "/./" |
1090 | if (strcmp(tok, ".") == 0) { | 1087 | if (strcmp(tok, ".") == 0) { |
1091 | tok = strtok(NULL, "/"); | 1088 | tok = strtok(NULL, "/"); |
1092 | continue; | 1089 | continue; |
1093 | } | 1090 | } |
1094 | weird_pathname = 0; | 1091 | // open the element, assuming it is a directory; this fails with ENOTDIR if it is a symbolic link |
1095 | |||
1096 | // open the directory | ||
1097 | fd = openat(parentfd, tok, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); | 1092 | fd = openat(parentfd, tok, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); |
1098 | close(parentfd); | ||
1099 | if (fd == -1) { | 1093 | if (fd == -1) { |
1094 | // if the following token is NULL, the current token is the final path element | ||
1095 | // try again to open it, this time using the passed flags, and return -1 or the descriptor | ||
1096 | current_tok = tok; | ||
1097 | tok = strtok(NULL, "/"); | ||
1098 | if (!tok) | ||
1099 | fd = openat(parentfd, current_tok, flags|O_NOFOLLOW); | ||
1100 | close(parentfd); | ||
1100 | free(dup); | 1101 | free(dup); |
1101 | return -1; | 1102 | return fd; // -1 if open failed |
1102 | } | 1103 | } |
1103 | 1104 | // move on to next path segment | |
1104 | parentfd = fd; | 1105 | current_tok = tok; |
1105 | tok = strtok(NULL, "/"); | 1106 | tok = strtok(NULL, "/"); |
1107 | if (tok) { | ||
1108 | close(parentfd); | ||
1109 | parentfd = fd; | ||
1110 | } | ||
1106 | } | 1111 | } |
1107 | if (p != dup) { | 1112 | |
1108 | // consistent flags for top level directories (////foo, /.///foo) | 1113 | // we are here because the last path element exists and is of file type directory |
1109 | if (weird_pathname) | 1114 | // reopen it using the passed flags |
1110 | flags = O_PATH|O_DIRECTORY|O_CLOEXEC; | 1115 | close(fd); |
1111 | // open last path segment | 1116 | fd = openat(parentfd, current_tok, flags|O_NOFOLLOW); |
1112 | fd = openat(parentfd, p + 1, flags|O_NOFOLLOW); | 1117 | close(parentfd); |
1113 | close(parentfd); | ||
1114 | } | ||
1115 | free(dup); | 1118 | free(dup); |
1116 | return fd; // -1 if open failed | 1119 | return fd; // -1 if open failed |
1117 | 1120 | ||