Merge pull request #5809 from keszybz/glob-safe

Implement `safe_glob` that ignores "." and ".."

9 files changed, 169 insertions, 53 deletions

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 <http://www.gnu.org/licenses/>.
 ***/
 
+#include <dirent.h>
 #include <errno.h>
 #include <glob.h>
+#include <sys/types.h>
 
+#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 <http://www.gnu.org/licenses/>.
 ***/
 
+#include <glob.h>
 #include <stdbool.h>
 #include <string.h>
 
 #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/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/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)
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 <fcntl.h>
+#include <glob.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 #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;
 }
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);