From 48d7c64805757d6c631a4ea2e973652f59058ac3 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Tue, 25 Apr 2017 23:44:34 -0400 Subject: basic: add readdir_no_dot and safe_glob functions safe_glob filters out "." and "..". This converts all users of glob_extend() and glob_exists() to safe_glob. --- src/basic/dirent-util.c | 11 ++++++++ src/basic/dirent-util.h | 2 ++ src/basic/glob-util.c | 64 ++++++++++++++++++++++++++++----------------- src/basic/glob-util.h | 4 +++ src/test/test-glob-util.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/basic/dirent-util.c b/src/basic/dirent-util.c index 6b9d26773e..5bf58bcdc3 100644 --- a/src/basic/dirent-util.c +++ b/src/basic/dirent-util.c @@ -75,3 +75,14 @@ bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) { return endswith(de->d_name, suffix); } + +struct dirent* readdir_no_dot(DIR *dirp) { + struct dirent* d; + + for (;;) { + d = readdir(dirp); + if (d && dot_or_dot_dot(d->d_name)) + continue; + return d; + } +} diff --git a/src/basic/dirent-util.h b/src/basic/dirent-util.h index b91d04908f..18b9db9b28 100644 --- a/src/basic/dirent-util.h +++ b/src/basic/dirent-util.h @@ -31,6 +31,8 @@ int dirent_ensure_type(DIR *d, struct dirent *de); bool dirent_is_file(const struct dirent *de) _pure_; bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) _pure_; +struct dirent* readdir_no_dot(DIR *dirp); + #define FOREACH_DIRENT(de, d, on_error) \ for (errno = 0, de = readdir(d);; errno = 0, de = readdir(d)) \ if (!de) { \ diff --git a/src/basic/glob-util.c b/src/basic/glob-util.c index 007198c269..f611c42e4c 100644 --- a/src/basic/glob-util.c +++ b/src/basic/glob-util.c @@ -17,54 +17,70 @@ along with systemd; If not, see . ***/ +#include #include #include +#include +#include "dirent-util.h" #include "glob-util.h" #include "macro.h" +#include "path-util.h" #include "strv.h" -int glob_exists(const char *path) { - _cleanup_globfree_ glob_t g = {}; +int safe_glob(const char *path, int flags, glob_t *pglob) { int k; - assert(path); + /* We want to set GLOB_ALTDIRFUNC ourselves, don't allow it to be set. */ + assert(!(flags & GLOB_ALTDIRFUNC)); + + if (!pglob->gl_closedir) + pglob->gl_closedir = (void (*)(void *)) closedir; + if (!pglob->gl_readdir) + pglob->gl_readdir = (struct dirent *(*)(void *)) readdir_no_dot; + if (!pglob->gl_opendir) + pglob->gl_opendir = (void *(*)(const char *)) opendir; + if (!pglob->gl_lstat) + pglob->gl_lstat = lstat; + if (!pglob->gl_stat) + pglob->gl_stat = stat; errno = 0; - k = glob(path, GLOB_NOSORT|GLOB_BRACE, NULL, &g); + k = glob(path, flags | GLOB_ALTDIRFUNC, NULL, pglob); if (k == GLOB_NOMATCH) - return 0; + return -ENOENT; if (k == GLOB_NOSPACE) return -ENOMEM; if (k != 0) return errno > 0 ? -errno : -EIO; + if (strv_isempty(pglob->gl_pathv)) + return -ENOENT; - return !strv_isempty(g.gl_pathv); + return 0; } -int glob_extend(char ***strv, const char *path) { +int glob_exists(const char *path) { _cleanup_globfree_ glob_t g = {}; int k; - char **p; - errno = 0; - k = glob(path, GLOB_NOSORT|GLOB_BRACE, NULL, &g); + assert(path); - if (k == GLOB_NOMATCH) - return -ENOENT; - if (k == GLOB_NOSPACE) - return -ENOMEM; - if (k != 0) - return errno > 0 ? -errno : -EIO; - if (strv_isempty(g.gl_pathv)) - return -ENOENT; + k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g); + if (k == -ENOENT) + return false; + if (k < 0) + return k; + return true; +} + +int glob_extend(char ***strv, const char *path) { + _cleanup_globfree_ glob_t g = {}; + int k; - STRV_FOREACH(p, g.gl_pathv) { - k = strv_extend(strv, *p); - if (k < 0) - return k; - } + k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g); + if (k < 0) + return k; - return 0; + return strv_extend_strv(strv, g.gl_pathv, false); } diff --git a/src/basic/glob-util.h b/src/basic/glob-util.h index 5d8fb47a26..e1f6083afa 100644 --- a/src/basic/glob-util.h +++ b/src/basic/glob-util.h @@ -19,12 +19,16 @@ along with systemd; If not, see . ***/ +#include #include #include #include "macro.h" #include "string-util.h" +/* Note: this function modifies pglob to set various functions. */ +int safe_glob(const char *path, int flags, glob_t *pglob); + int glob_exists(const char *path); int glob_extend(char ***strv, const char *path); diff --git a/src/test/test-glob-util.c b/src/test/test-glob-util.c index 9eea3eb608..af866e004b 100644 --- a/src/test/test-glob-util.c +++ b/src/test/test-glob-util.c @@ -18,12 +18,17 @@ ***/ #include +#include +#include #include #include "alloc-util.h" +#include "dirent-util.h" #include "fileio.h" +#include "fs-util.h" #include "glob-util.h" #include "macro.h" +#include "rm-rf.h" static void test_glob_exists(void) { char name[] = "/tmp/test-glob_exists.XXXXXX"; @@ -43,8 +48,69 @@ static void test_glob_exists(void) { assert_se(r == 0); } +static void test_glob_no_dot(void) { + char template[] = "/tmp/test-glob-util.XXXXXXX"; + const char *fn; + + _cleanup_globfree_ glob_t g = { + .gl_closedir = (void (*)(void *)) closedir, + .gl_readdir = (struct dirent *(*)(void *)) readdir_no_dot, + .gl_opendir = (void *(*)(const char *)) opendir, + .gl_lstat = lstat, + .gl_stat = stat, + }; + + int r; + + assert_se(mkdtemp(template)); + + fn = strjoina(template, "/*"); + r = glob(fn, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g); + assert_se(r == GLOB_NOMATCH); + + fn = strjoina(template, "/.*"); + r = glob(fn, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g); + assert_se(r == GLOB_NOMATCH); + + (void) rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL); +} + +static void test_safe_glob(void) { + char template[] = "/tmp/test-glob-util.XXXXXXX"; + const char *fn, *fn2, *fname; + + _cleanup_globfree_ glob_t g = {}; + int r; + + assert_se(mkdtemp(template)); + + fn = strjoina(template, "/*"); + r = safe_glob(fn, 0, &g); + assert_se(r == -ENOENT); + + fn2 = strjoina(template, "/.*"); + r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g); + assert_se(r == -ENOENT); + + fname = strjoina(template, "/.foobar"); + assert_se(touch(fname) == 0); + + r = safe_glob(fn, 0, &g); + assert_se(r == -ENOENT); + + r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g); + assert_se(r == 0); + assert_se(g.gl_pathc == 1); + assert_se(streq(g.gl_pathv[0], fname)); + assert_se(g.gl_pathv[1] == NULL); + + (void) rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL); +} + int main(void) { test_glob_exists(); + test_glob_no_dot(); + test_safe_glob(); return 0; } -- cgit v1.2.3-54-g00ecf From 84e72b5ef445ffb256bc4add4209c4c9c9855206 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Tue, 25 Apr 2017 23:50:35 -0400 Subject: tmpfiles: use safe_glob() This filters out "." and ".." from glob results. Fixes #5655 and #5644. Any judgements on whether the path is "safe" are removed. We will not remove "/" under any name (including "/../" and such), but we will remove stuff that is specified using paths that include "//", "/./" and "/../". Such paths can be created when joining strings automatically, or for other reasons, and people generally know what ".." and "." is. Tests are added to make sure that the helper functions behave as expected. --- src/basic/rm-rf.c | 9 +-------- src/test/test-path-util.c | 34 ++++++++++++++++++++++++++++++++++ src/tmpfiles/tmpfiles.c | 11 +++-------- 3 files changed, 38 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index bdaca264ff..ff040e7a55 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -182,18 +182,11 @@ int rm_rf(const char *path, RemoveFlags flags) { /* We refuse to clean the root file system with this * call. This is extra paranoia to never cause a really * seriously broken system. */ - if (path_equal(path, "/")) { + if (path_equal_or_files_same(path, "/")) { log_error("Attempted to remove entire root file system, and we can't allow that."); return -EPERM; } - /* Another safe-check. Removing "/path/.." could easily remove entire root as well. - * It's especially easy to do using globs in tmpfiles, like "/path/.*", which the glob() - * function expands to both "/path/." and "/path/..". - * Return -EINVAL to be consistent with rmdir("/path/."). */ - if (endswith(path, "/..") || endswith(path, "/../")) - return -EINVAL; - if ((flags & (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) == (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) { /* Try to remove as subvolume first */ r = btrfs_subvol_remove(path, BTRFS_REMOVE_RECURSIVE|BTRFS_REMOVE_QUOTA); diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 22df20a1eb..ab62d0dad2 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -27,6 +27,7 @@ #include "mount-util.h" #include "path-util.h" #include "rm-rf.h" +#include "stat-util.h" #include "string-util.h" #include "strv.h" #include "util.h" @@ -104,6 +105,38 @@ static void test_path(void) { assert_se(!path_equal_ptr(NULL, "/a")); } +static void test_path_equal_root(void) { + /* Nail down the details of how path_equal("/", ...) works. */ + + assert_se(path_equal("/", "/")); + assert_se(path_equal("/", "//")); + + assert_se(!path_equal("/", "/./")); + assert_se(!path_equal("/", "/../")); + + assert_se(!path_equal("/", "/.../")); + + /* Make sure that files_same works as expected. */ + + assert_se(files_same("/", "/") > 0); + assert_se(files_same("/", "//") > 0); + + assert_se(files_same("/", "/./") > 0); + assert_se(files_same("/", "/../") > 0); + + assert_se(files_same("/", "/.../") == -ENOENT); + + /* The same for path_equal_or_files_same. */ + + assert_se(path_equal_or_files_same("/", "/")); + assert_se(path_equal_or_files_same("/", "//")); + + assert_se(path_equal_or_files_same("/", "/./")); + assert_se(path_equal_or_files_same("/", "/../")); + + assert_se(!path_equal_or_files_same("/", "/.../")); +} + static void test_find_binary(const char *self) { char *p; @@ -551,6 +584,7 @@ int main(int argc, char **argv) { log_open(); test_path(); + test_path_equal_root(); test_find_binary(argv[0]); test_prefixes(); test_path_join(); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index ed6a9adaa6..36488c9a5c 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1093,19 +1093,14 @@ static int item_do_children(Item *i, const char *path, action_t action) { static int glob_item(Item *i, action_t action, bool recursive) { _cleanup_globfree_ glob_t g = { - .gl_closedir = (void (*)(void *)) closedir, - .gl_readdir = (struct dirent *(*)(void *)) readdir, .gl_opendir = (void *(*)(const char *)) opendir_nomod, - .gl_lstat = lstat, - .gl_stat = stat, }; int r = 0, k; char **fn; - errno = 0; - k = glob(i->path, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g); - if (k != 0 && k != GLOB_NOMATCH) - return log_error_errno(errno ?: EIO, "glob(%s) failed: %m", i->path); + k = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g); + if (k < 0 && k != -ENOENT) + return log_error_errno(k, "glob(%s) failed: %m", i->path); STRV_FOREACH(fn, g.gl_pathv) { k = action(i, *fn); -- cgit v1.2.3-54-g00ecf From d8c92e8bc7351f553936b5235e1922c18ebd817a Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Tue, 25 Apr 2017 22:54:50 -0400 Subject: execute: filter out "." for ".." in EnvironmentFile= globs too This doesn't really matter much, only in case somebody would use something strange like EnvironmentFile=/etc/something/.* Make sure that "." and ".." is not returned by that glob. This makes all our globbing patterns behave the same. --- src/core/execute.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/core/execute.c b/src/core/execute.c index 2056e2273c..9c1aa4cf98 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3231,11 +3231,10 @@ int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) { STRV_FOREACH(i, c->environment_files) { char *fn; - int k; + int k, n; bool ignore = false; char **p; _cleanup_globfree_ glob_t pglob = {}; - int count, n; fn = *i; @@ -3253,23 +3252,19 @@ int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) { } /* Filename supports globbing, take all matching files */ - errno = 0; - if (glob(fn, 0, NULL, &pglob) != 0) { + k = safe_glob(fn, 0, &pglob); + if (k < 0) { if (ignore) continue; strv_free(r); - return errno > 0 ? -errno : -EINVAL; + return k; } - count = pglob.gl_pathc; - if (count == 0) { - if (ignore) - continue; - strv_free(r); - return -EINVAL; - } - for (n = 0; n < count; n++) { + /* When we don't match anything, -ENOENT should be returned */ + assert(pglob.gl_pathc > 0); + + for (n = 0; n < pglob.gl_pathc; n++) { k = load_env_file(NULL, pglob.gl_pathv[n], NULL, &p); if (k < 0) { if (ignore) -- cgit v1.2.3-54-g00ecf