From a4eaf3cf822dae1d076dfc98afc3ab0d53871dac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 29 Nov 2016 15:54:42 +0100 Subject: [PATCH] fs-util: change chase_symlinks() behaviour in regards to escaping the root dir Previously, we'd generate an EINVAL error if it is attempted to escape a root directory with relative ".." symlinks. With this commit this is changed so that ".." from the root directory is a NOP, following the kernel's own behaviour where /.. is equivalent to /. As suggested by @keszybz. --- src/basic/fs-util.c | 12 +++++++----- src/test/test-fs-util.c | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 30e1b2a674..c20faf67b0 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -611,8 +611,8 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { * symlinks relative to a root directory, instead of the root of the host. * * Note that "root" primarily matters if we encounter an absolute symlink. It is also used when following - * relative symlinks to ensure they cannot be used to "escape" the root directory. (For cases where this is - * attempted -EINVAL is returned.). The path parameter passed shall *not* be prefixed by it. + * relative symlinks to ensure they cannot be used to "escape" the root directory. The path parameter passed + * shall *not* be prefixed by it. * * Algorithmically this operates on two path buffers: "done" are the components of the path we already * processed and resolved symlinks, "." and ".." of. "todo" are the components of the path we still need to @@ -674,18 +674,20 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { _cleanup_free_ char *parent = NULL; int fd_parent = -1; + /* If we already are at the top, then going up will not change anything. This is in-line with + * how the kernel handles this. */ if (isempty(done) || path_equal(done, "/")) - return -EINVAL; + continue; parent = dirname_malloc(done); if (!parent) return -ENOMEM; - /* Don't allow this to leave the root dir */ + /* Don't allow this to leave the root dir. */ if (root && path_startswith(done, root) && !path_startswith(parent, root)) - return -EINVAL; + continue; free_and_replace(done, parent); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 7c6deb5416..792ad46847 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -97,21 +97,21 @@ static void test_chase_symlinks(void) { result = mfree(result); r = chase_symlinks(p, temp, &result); - assert_se(r == -EINVAL); + assert_se(r == 0 && path_equal(result, temp)); p = strjoina(temp, "/6dotsusr"); assert_se(symlink("../../../usr", p) >= 0); result = mfree(result); r = chase_symlinks(p, temp, &result); - assert_se(r == -EINVAL); + assert_se(r == 0 && path_equal(result, q)); p = strjoina(temp, "/top/8dotsusr"); assert_se(symlink("../../../../usr", p) >= 0); result = mfree(result); r = chase_symlinks(p, temp, &result); - assert_se(r == -EINVAL); + assert_se(r == 0 && path_equal(result, q)); /* Paths that contain repeated slashes */ @@ -137,7 +137,7 @@ static void test_chase_symlinks(void) { result = mfree(result); r = chase_symlinks("/etc/./.././", "/etc", &result); - assert_se(r == -EINVAL); + assert_se(r == 0 && path_equal(result, "/etc")); result = mfree(result); r = chase_symlinks("/etc/machine-id/foo", NULL, &result);