]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
Merge pull request #26341 from DaanDeMeyer/chase-fixes
authorLuca Boccassi <bluca@debian.org>
Fri, 17 Feb 2023 11:44:47 +0000 (11:44 +0000)
committerGitHub <noreply@github.com>
Fri, 17 Feb 2023 11:44:47 +0000 (11:44 +0000)
chase-symlinks fixes

src/basic/chase-symlinks.c
src/test/test-fs-util.c

index 4cc0a01df8ef0a289b8ef034661ed5574fe353f6..3a4fdcd1e0247b4e53b359b2b9a7a2cab2ebacf6 100644 (file)
@@ -102,20 +102,22 @@ int chase_symlinks_at(
                 path = ".";
 
         /* This function resolves symlinks of the path relative to the given directory file descriptor. If
-         * CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified, symlinks are resolved relative to the given directory
-         * file descriptor. Otherwise, they are resolved relative to the root directory of the host.
+         * CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified and a directory file descriptor is provided, symlinks
+         * are resolved relative to the given directory file descriptor. Otherwise, they are resolved
+         * relative to the root directory of the host.
          *
-         * Note that when CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified and we find an absolute symlink, it is
-         * resolved relative to given directory file descriptor and not the root of the host. Also, when
-         * following relative symlinks, this functions ensure they cannot be used to "escape" the given
-         * directory file descriptor. The "path" parameter is always interpreted relative to the given
-         * directory file descriptor. If the given directory file descriptor is AT_FDCWD and "path" is
-         * absolute, it is interpreted relative to the root directory of the host.
+         * Note that when a positive directory file descriptor is provided and CHASE_AT_RESOLVE_IN_ROOT is
+         * specified and we find an absolute symlink, it is resolved relative to given directory file
+         * descriptor and not the root of the host. Also, when following relative symlinks, this functions
+         * ensures they cannot be used to "escape" the given directory file descriptor. If a positive
+         * directory file descriptor is provided, the "path" parameter is always interpreted relative to the
+         * given directory file descriptor, even if it is absolute. If the given directory file descriptor is
+         * AT_FDCWD and "path" is absolute, it is interpreted relative to the root directory of the host.
          *
          * If "dir_fd" is a valid directory fd, "path" is an absolute path and "ret_path" is not NULL, this
          * functions returns a relative path in "ret_path" because openat() like functions generally ignore
          * the directory fd if they are provided with an absolute path. On the other hand, if "dir_fd" is
-         * AT_FDCWD and "path" is an absolute path, we need to return an absolute path in "ret_path" because
+         * AT_FDCWD and "path" is an absolute path, we return an absolute path in "ret_path" because
          * otherwise, if the caller passes the returned relative path to another openat() like function, it
          * would be resolved relative to the current working directory instead of to "/".
          *
@@ -177,21 +179,30 @@ int chase_symlinks_at(
         if (!buffer)
                 return -ENOMEM;
 
-        bool need_absolute = !FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT) && path_is_absolute(path);
+        /* If we receive an absolute path together with AT_FDCWD, we need to return an absolute path, because
+         * a relative path would be interpreted relative to the current working directory. */
+        bool need_absolute = dir_fd == AT_FDCWD && path_is_absolute(path);
         if (need_absolute) {
                 done = strdup("/");
                 if (!done)
                         return -ENOMEM;
         }
 
-        if (FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT))
+        /* If we get AT_FDCWD, we always resolve symlinks relative to the host's root. Only if a positive
+         * directory file descriptor is provided will we look at CHASE_AT_RESOLVE_IN_ROOT to determine
+         * whether to resolve symlinks in it or not. */
+        if (dir_fd >= 0 && FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT))
                 root_fd = openat(dir_fd, ".", O_CLOEXEC|O_DIRECTORY|O_PATH);
         else
                 root_fd = open("/", O_CLOEXEC|O_DIRECTORY|O_PATH);
         if (root_fd < 0)
                 return -errno;
 
-        if (FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT) || !path_is_absolute(path))
+        /* If a positive directory file descriptor is provided, always resolve the given path relative to it,
+         * regardless of whether it is absolute or not. If we get AT_FDCWD, follow regular openat()
+         * semantics, if the path is relative, resolve against the current working directory. Otherwise,
+         * resolve against root. */
+        if (dir_fd >= 0 || !path_is_absolute(path))
                 fd = openat(dir_fd, ".", O_CLOEXEC|O_DIRECTORY|O_PATH);
         else
                 fd = open("/", O_CLOEXEC|O_DIRECTORY|O_PATH);
@@ -453,30 +464,26 @@ int chase_symlinks(
                         return r;
         }
 
-        if (root) {
-                path = path_startswith(absolute, root);
-                if (!path)
-                        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.",
-                                              absolute, root);
+        path = path_startswith(absolute, empty_to_root(root));
+        if (!path)
+                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.",
+                                        absolute, empty_to_root(root));
 
-                fd = open(root, O_CLOEXEC|O_DIRECTORY|O_PATH);
-                if (fd < 0)
-                        return -errno;
+        fd = open(empty_to_root(root), O_CLOEXEC|O_DIRECTORY|O_PATH);
+        if (fd < 0)
+                return -errno;
 
-                flags |= CHASE_AT_RESOLVE_IN_ROOT;
-        } else {
-                path = absolute;
-                fd = AT_FDCWD;
-        }
+        flags |= CHASE_AT_RESOLVE_IN_ROOT;
+        flags &= ~CHASE_PREFIX_ROOT;
 
-        r = chase_symlinks_at(fd, path, flags & ~CHASE_PREFIX_ROOT, ret_path ? &p : NULL, ret_fd ? &pfd : NULL);
+        r = chase_symlinks_at(fd, path, flags, ret_path ? &p : NULL, ret_fd ? &pfd : NULL);
         if (r < 0)
                 return r;
 
         if (ret_path) {
-                char *q = path_join(root, p);
+                char *q = path_join(empty_to_root(root), p);
                 if (!q)
                         return -ENOMEM;
 
index 668a44733bc93a65b525e34d94336131bd9c152e..3fc5f88d26e1e9299052e3a7c5faca9dd9c096fa 100644 (file)
@@ -430,6 +430,42 @@ TEST(chase_symlinks) {
         assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
 }
 
+TEST(chase_symlinks_at) {
+        _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
+        _cleanup_close_ int tfd = -EBADF, fd = -EBADF;
+        _cleanup_free_ char *result = NULL;
+        const char *p;
+
+        assert_se((tfd = mkdtemp_open(NULL, 0, &t)) >= 0);
+
+        /* Test that AT_FDCWD with CHASE_AT_RESOLVE_IN_ROOT resolves against / and not the current working
+         * directory. */
+
+        assert_se(symlinkat("/usr", tfd, "abc") >= 0);
+
+        p = strjoina(t, "/abc");
+        assert_se(chase_symlinks_at(AT_FDCWD, p, CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0);
+        assert_se(streq(result, "/usr"));
+        result = mfree(result);
+
+        /* Test that absolute path or not are the same when resolving relative to a directory file
+         * descriptor and that we always get a relative path back. */
+
+        assert_se(fd = openat(tfd, "def", O_CREAT|O_CLOEXEC, 0700) >= 0);
+        fd = safe_close(fd);
+        assert_se(symlinkat("/def", tfd, "qed") >= 0);
+        assert_se(chase_symlinks_at(tfd, "qed", CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0);
+        assert_se(streq(result, "def"));
+        result = mfree(result);
+        assert_se(chase_symlinks_at(tfd, "/qed", CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0);
+        assert_se(streq(result, "def"));
+        result = mfree(result);
+
+        /* Valid directory file descriptor without CHASE_AT_RESOLVE_IN_ROOT should resolve symlinks against
+         * host's root. */
+        assert_se(chase_symlinks_at(tfd, "/qed", 0, &result, NULL) == -ENOENT);
+}
+
 TEST(unlink_noerrno) {
         char *name;
         int fd;