From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 1 Nov 2023 11:46:17 +0000 (+0100)
Subject: chase: fix corner case when using CHASE_PARENT with a path ending in ".."
X-Git-Tag: v255-rc1~59
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9c21cfdd7d68797c6ae63ca20aa624577ba05246;p=thirdparty%2Fsystemd.git

chase: fix corner case when using CHASE_PARENT with a path ending in ".."

If we use CHASE_PARENT on a path ending in ".." then things are a bit
weird, because we the last path we look at is actually the *parent* and not
the *child* of the preceeding path. Hence we cannot just return the 2nd
to last fd we look at. We have to correct it, by going *two* levels up,
to get to the actual parent, and make sure CHASE_PARENT does what it
should.

Example: for the path /a/b/c chase() with CHASE_PARENT will return
/a/b/c as path, and the fd returned points to /a/b. All good.  But now,
for the path /a/b/c/.. chase() with CHASE_PARENT would previously return
/a/b as path (which is OK) but the fd would point to /a/b/c, which is
*not* the parent of /a/b, after all! To get to the actual parent of
/a/b we have to go *two* levels up to get to /a.

Very confusing. But that's what we here for, no?

@mrc0mmand ran into this in https://github.com/systemd/systemd/pull/28891#issuecomment-1782833722
---

diff --git a/src/basic/chase.c b/src/basic/chase.c
index c3ecbbb3fee..b0592c538dd 100644
--- a/src/basic/chase.c
+++ b/src/basic/chase.c
@@ -335,8 +335,28 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int
                             unsafe_transition(&st, &st_parent))
                                 return log_unsafe_transition(fd, fd_parent, path, flags);
 
-                        if (FLAGS_SET(flags, CHASE_PARENT) && isempty(todo))
+                        /* If the path ends on a "..", and CHASE_PARENT is specified then our current 'fd' is
+                         * the child of the returned normalized path, not the parent as requested. To correct
+                         * this we have to go *two* levels up. */
+                        if (FLAGS_SET(flags, CHASE_PARENT) && isempty(todo)) {
+                                _cleanup_close_ int fd_grandparent = -EBADF;
+                                struct stat st_grandparent;
+
+                                fd_grandparent = openat(fd_parent, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH|O_DIRECTORY);
+                                if (fd_grandparent < 0)
+                                        return -errno;
+
+                                if (fstat(fd_grandparent, &st_grandparent) < 0)
+                                        return -errno;
+
+                                if (FLAGS_SET(flags, CHASE_SAFE) &&
+                                    unsafe_transition(&st_parent, &st_grandparent))
+                                        return log_unsafe_transition(fd_parent, fd_grandparent, path, flags);
+
+                                st = st_grandparent;
+                                close_and_replace(fd, fd_grandparent);
                                 break;
+                        }
 
                         /* update fd and stat */
                         st = st_parent;
diff --git a/src/test/test-chase.c b/src/test/test-chase.c
index d3399c11c6c..dbbc99bf81b 100644
--- a/src/test/test-chase.c
+++ b/src/test/test-chase.c
@@ -721,6 +721,33 @@ TEST(chaseat_prefix_root) {
         assert_se(streq(ret, expected));
 }
 
+TEST(trailing_dot_dot) {
+        _cleanup_free_ char *path = NULL, *fdpath = NULL;
+        _cleanup_close_ int fd = -EBADF;
+
+        assert_se(chase("/usr/..", NULL, CHASE_PARENT, &path, &fd) >= 0);
+        assert_se(path_equal(path, "/"));
+        assert_se(fd_get_path(fd, &fdpath) >= 0);
+        assert_se(path_equal(fdpath, "/"));
+
+        path = mfree(path);
+        fdpath = mfree(fdpath);
+        fd = safe_close(fd);
+
+        _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
+        assert_se(mkdtemp_malloc(NULL, &t) >= 0);
+        _cleanup_free_ char *sub = ASSERT_PTR(path_join(t, "a/b/c/d"));
+        assert_se(mkdir_p(sub, 0700) >= 0);
+        _cleanup_free_ char *suffixed = ASSERT_PTR(path_join(sub, ".."));
+        assert_se(chase(suffixed, NULL, CHASE_PARENT, &path, &fd) >= 0);
+        _cleanup_free_ char *expected1 = ASSERT_PTR(path_join(t, "a/b/c"));
+        _cleanup_free_ char *expected2 = ASSERT_PTR(path_join(t, "a/b"));
+
+        assert_se(path_equal(path, expected1));
+        assert_se(fd_get_path(fd, &fdpath) >= 0);
+        assert_se(path_equal(fdpath, expected2));
+}
+
 static int intro(void) {
         arg_test_dir = saved_argv[1];
         return EXIT_SUCCESS;