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.

diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c
index bdaca264ffb8e161a652b0379b0dee78f8726d50..ff040e7a5516c24a158597b7e2e01e5e9c051203 100644 (file)
--- 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 22df20a1eb9ceaa6dc82be0a0100676501e355e2..ab62d0dad2f043da6674b1973bb1e364554616f2 100644 (file)
--- 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 ed6a9adaa614a36ff1a4be26c13df2c5c19a3868..36488c9a5c2a6b08daf0b1bf5b2c6d184e6923b0 100644 (file)
--- 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);