]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
fs-util: change chase_symlinks() behaviour in regards to escaping the root dir
authorLennart Poettering <lennart@poettering.net>
Tue, 29 Nov 2016 14:54:42 +0000 (15:54 +0100)
committerLennart Poettering <lennart@poettering.net>
Wed, 30 Nov 2016 23:25:51 +0000 (00:25 +0100)
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
src/test/test-fs-util.c

index 30e1b2a674f7dcd702ae4127a803beb4eced51b3..c20faf67b02658c1c1c2b7103ba548fc65c88313 100644 (file)
@@ -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);
 
index 7c6deb5416871feac267b06d542c6f54a13d48dc..792ad46847fbf57503bee7b912a46aaac3ba8114 100644 (file)
@@ -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);