From c2595d3b02849b7baa94483f03cbc888e0c63ebd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Jan 2020 21:02:29 +0100 Subject: fs-util: when calling chase_symlinks() with root path, leave root part unresolved Previously there was a weird asymmetry: initially we'd resolve the specified prefix path when chasing symlinks together with the actual path we were supposed to cover, except when we hit an absolute symlink where we'd use the root as it was. Let's unify handling here: the prefix path is never resolved, and always left as it is. This in particular fixes issues with symlinks in the prefix path, as that confused the check that made sure we never left the root directory. Fixes: #14634 Replaces: #14635 --- src/basic/fs-util.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 5723c845e43..5ec32854c30 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -810,7 +810,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 +819,33 @@ 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); + + /* Make sure "done" ends without a slash */ + done = strdup(root); + if (!done) + return -ENOMEM; + delete_trailing_chars(done, "/"); + + /* 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; @@ -930,7 +957,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 +987,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; @@ -984,6 +1010,8 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, done = strdup(root); if (!done) return -ENOMEM; + + delete_trailing_chars(done, "/"); } /* Prefix what's left to do with what we just read, and start the loop again, but -- cgit v1.2.3-59-g8ed1b From 6efb1257d10cd1ecdae6208d083af36f15e4c05f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Jan 2020 21:40:58 +0100 Subject: test: add test for the non-resolving of chase_symlink() root prefix --- src/test/test-fs-util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index ac8b95aece0..d5492d937b9 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -371,6 +371,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); } -- cgit v1.2.3-59-g8ed1b From 47d7ab727cf5bd0697b17854a948971101b4e725 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Jan 2020 21:56:10 +0100 Subject: fs-util: make sure we output normalized paths in chase_symlinks() Let's eat up multiple slashes. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1787089 Replaces: #14687 --- src/basic/fs-util.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 5ec32854c30..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)) @@ -832,11 +840,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, "Specified path '%s' is outside of specified root directory '%s', refusing to resolve.", path, root); - /* Make sure "done" ends without a slash */ done = strdup(root); if (!done) return -ENOMEM; - delete_trailing_chars(done, "/"); /* Make sure "todo" starts with a slash */ absolute = strjoin("/", e); @@ -855,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. */ @@ -1010,8 +1025,6 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, done = strdup(root); if (!done) return -ENOMEM; - - delete_trailing_chars(done, "/"); } /* Prefix what's left to do with what we just read, and start the loop again, but -- cgit v1.2.3-59-g8ed1b From 3c7b4ebf94d167df835c4a5a21b813f803c23b05 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Jan 2020 22:00:02 +0100 Subject: test: make sure chase_symlink() returns normalized paths --- src/test/test-fs-util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index d5492d937b9..1a5fd56658f 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); -- cgit v1.2.3-59-g8ed1b From bcb1eadc0cf8053d219c0b2cab1c46235506981d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Jan 2020 21:40:03 +0100 Subject: test: fix rename_noreplace() test This corrects the fix b81b9d406de, making the test operate like it was originally. --- src/test/test-fs-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 1a5fd56658f..d0c6fb82bfd 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -739,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) { @@ -747,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"); -- cgit v1.2.3-59-g8ed1b