Merge pull request #14689 from poettering/portable-chase-symlink-fix

various chase_symlink() fixes

2 files changed, 56 insertions, 5 deletions

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 5723c845e43..f8095e85d82 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -797,6 +797,14 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                 if (r < 0)
                         return r;
 
+                /* Simplify the root directory, so that it has no duplicate slashes and nothing at the
+                 * end. While we won't resolve the root path we still simplify it. Note that dropping the
+                 * trailing slash should not change behaviour, since when opening it we specify O_DIRECTORY
+                 * anyway. Moreover at the end of this function after processing everything we'll always turn
+                 * the empty string back to "/". */
+                delete_trailing_chars(root, "/");
+                path_simplify(root, true);
+
                 if (flags & CHASE_PREFIX_ROOT) {
                         /* We don't support relative paths in combination with a root directory */
                         if (!path_is_absolute(path))
@@ -810,7 +818,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
         if (r < 0)
                 return r;
 
-        fd = open("/", O_CLOEXEC|O_NOFOLLOW|O_PATH);
+        fd = open(root ?: "/", O_CLOEXEC|O_DIRECTORY|O_PATH);
         if (fd < 0)
                 return -errno;
 
@@ -819,6 +827,31 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                         return -errno;
         }
 
+        if (root) {
+                _cleanup_free_ char *absolute = NULL;
+                const char *e;
+
+                /* If we are operating on a root directory, let's take the root directory as it is. */
+
+                e = path_startswith(buffer, root);
+                if (!e)
+                        return log_full_errno(flags & CHASE_WARN ? LOG_WARNING : LOG_DEBUG,
+                                              SYNTHETIC_ERRNO(ECHRNG),
+                                              "Specified path '%s' is outside of specified root directory '%s', refusing to resolve.",
+                                              path, root);
+
+                done = strdup(root);
+                if (!done)
+                        return -ENOMEM;
+
+                /* Make sure "todo" starts with a slash */
+                absolute = strjoin("/", e);
+                if (!absolute)
+                        return -ENOMEM;
+
+                free_and_replace(buffer, absolute);
+        }
+
         todo = buffer;
         for (;;) {
                 _cleanup_free_ char *first = NULL;
@@ -828,6 +861,15 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                 /* Determine length of first component in the path */
                 n = strspn(todo, "/");                  /* The slashes */
+
+                if (n > 1) {
+                        /* If we are looking at more than a single slash then skip all but one, so that when
+                         * we are done with everything we have a normalized path with only single slashes
+                         * separating the path components. */
+                        todo += n - 1;
+                        n = 1;
+                }
+
                 m = n + strcspn(todo + n, "/");         /* The entire length of the component */
 
                 /* Extract the first component. */
@@ -930,7 +972,6 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                 if (fstat(child, &st) < 0)
                         return -errno;
                 if ((flags & CHASE_SAFE) &&
-                    (empty_or_root(root) || (size_t)(todo - buffer) > strlen(root)) &&
                     unsafe_transition(&previous_stat, &st))
                         return log_unsafe_transition(fd, child, path, flags);
 
@@ -961,7 +1002,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                                  * directory as base. */
 
                                 safe_close(fd);
-                                fd = open(root ?: "/", O_CLOEXEC|O_NOFOLLOW|O_PATH);
+                                fd = open(root ?: "/", O_CLOEXEC|O_DIRECTORY|O_PATH);
                                 if (fd < 0)
                                         return -errno;
 
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index ac8b95aece0..d0c6fb82bfd 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -148,6 +148,7 @@ static void test_chase_symlinks(void) {
         r = chase_symlinks(p, NULL, 0, &result, NULL);
         assert_se(r > 0);
         assert_se(path_equal(result, "/usr"));
+        assert_se(streq(result, "/usr")); /* we guarantee that we drop redundant slashes */
         result = mfree(result);
 
         r = chase_symlinks(p, temp, 0, &result, NULL);
@@ -371,6 +372,15 @@ static void test_chase_symlinks(void) {
         assert_se(streq("/usr", result));
         result = mfree(result);
 
+        /* Make sure that symlinks in the "root" path are not resolved, but those below are */
+        p = strjoina("/etc/..", temp, "/self");
+        assert_se(symlink(".", p) >= 0);
+        q = strjoina(p, "/top/dot/dotdota");
+        r = chase_symlinks(q, p, 0, &result, NULL);
+        assert_se(r > 0);
+        assert_se(path_equal(path_startswith(result, p), "usr"));
+        result = mfree(result);
+
  cleanup:
         assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
 }
@@ -729,7 +739,7 @@ static void test_rename_noreplace(void) {
                 STRV_FOREACH(b, (char**) table) {
                         _cleanup_free_ char *w = NULL;
 
-                        w = strjoin(w, *b);
+                        w = strjoin(z, *b);
                         assert_se(w);
 
                         if (access(w, F_OK) < 0) {
@@ -737,7 +747,7 @@ static void test_rename_noreplace(void) {
                                 continue;
                         }
 
-                        assert_se(rename_noreplace(AT_FDCWD, w, AT_FDCWD, y) == -EEXIST);
+                        assert_se(rename_noreplace(AT_FDCWD, x, AT_FDCWD, w) == -EEXIST);
                 }
 
                 y = strjoin(z, "/somethingelse");