Fix handling of .. paths in symlinks. (#366)

* Fix handling of `..` paths in symlinks.

When `..` appears at the end of a symlink target, but is not at the end
of the full path target, don't mark the path as being expected to open
a directory.

This fixes the reduced testcase in bytecodealliance/wasmtime#9272.

* Update CI to macos-12.

Author: sunfishcode

.github/workflows/main.yml

@@ -201,7 +201,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        build: [stable, windows-latest, windows-2019, macos-latest, macos-11, beta, ubuntu-20.04, aarch64-ubuntu]
+        build: [stable, windows-latest, windows-2019, macos-latest, macos-12, beta, ubuntu-20.04, aarch64-ubuntu]
         include:
           - build: stable
             os: ubuntu-latest

cap-primitives/src/fs/manually/open.rs

@@ -314,6 +314,7 @@ impl<'start> Context<'start> {
 
     /// Push the components of `destination` onto the worklist stack.
     fn push_symlink_destination(&mut self, destination: PathBuf) -> io::Result<()> {
+        let at_end = self.components.is_empty();
         let trailing_slash = path_has_trailing_slash(&destination);
         let trailing_dot = path_has_trailing_dot(&destination);
         let trailing_dotdot = destination.ends_with(Component::ParentDir);
@@ -362,9 +363,11 @@ impl<'start> Context<'start> {
 
         // Record whether the new components ended with a path that implies
         // an open of `.` at the end of path resolution.
-        self.follow_with_dot |= trailing_dot | trailing_dotdot;
-        self.trailing_slash |= trailing_slash;
-        self.dir_required |= trailing_slash;
+        if at_end {
+            self.follow_with_dot |= trailing_dot | trailing_dotdot;
+            self.trailing_slash |= trailing_slash;
+            self.dir_required |= trailing_slash;
+        }
 
         // As an optimization, hold onto the `PathBuf` buffer for later reuse.
         self.reuse = destination;

tests/fs_additional.rs

@@ -5,36 +5,28 @@
 #[macro_use]
 mod sys_common;
 
-use cap_std::fs::{DirBuilder, OpenOptions};
+use cap_std::fs::{Dir, DirBuilder, OpenOptions};
 use cap_std::time::SystemClock;
 use std::io::{self, Read, Write};
 use std::path::Path;
 use std::str;
-use sys_common::io::{tmpdir, TempDir};
+use sys_common::io::tmpdir;
 use sys_common::symlink_supported;
 
 #[cfg(not(windows))]
-fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> {
+fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &Dir, dst: Q) -> io::Result<()> {
     tmpdir.symlink(src, dst)
 }
 #[cfg(not(windows))]
-fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(
-    src: P,
-    tmpdir: &TempDir,
-    dst: Q,
-) -> io::Result<()> {
+fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &Dir, dst: Q) -> io::Result<()> {
     tmpdir.symlink(src, dst)
 }
 #[cfg(windows)]
-fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> {
+fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &Dir, dst: Q) -> io::Result<()> {
     tmpdir.symlink_dir(src, dst)
 }
 #[cfg(windows)]
-fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(
-    src: P,
-    tmpdir: &TempDir,
-    dst: Q,
-) -> io::Result<()> {
+fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(src: P, tmpdir: &Dir, dst: Q) -> io::Result<()> {
     tmpdir.symlink_file(src, dst)
 }
 
@@ -1046,3 +1038,321 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) {
         assert_eq!(std.blocks(), cap_std::fs::MetadataExt::blocks(cap));
     }
 }
+
+/// Test that a symlink in the middle of a path containing ".." doesn't cause
+/// the path to be treated as if it ends with "..".
+#[test]
+fn dotdot_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir("b"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("..", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Like `dotdot_in_middle_of_symlink` but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir("b"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("../.", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Same as `dotdot_in_middle_of_symlink`, but use two levels of `..`.
+///
+/// Windows doesn't appear to like symlinks that end with `/..`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_more_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir_all("b/c"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("c/../..", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Like `dotdot_more_in_middle_of_symlink`, but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_more_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir_all("b/c"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("c/../../.", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Same as `dotdot_more_in_middle_of_symlink`, but the symlink doesn't
+/// include `c`.
+///
+/// Windows doesn't appear to like symlinks that end with `/..`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_other_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir_all("b/c"));
+    let c = check!(tmpdir.open_dir("b/c"));
+    check!(symlink_dir("../..", &c, "up"));
+
+    let path = "b/c/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Like `dotdot_other_in_middle_of_symlink`, but with `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_other_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir_all("b/c"));
+    let c = check!(tmpdir.open_dir("b/c"));
+    check!(symlink_dir("../../.", &c, "up"));
+
+    let path = "b/c/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Same as `dotdot_more_in_middle_of_symlink`, but use a symlink that
+/// doesn't end with `..`.
+#[test]
+fn dotdot_even_more_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir_all("b/c"));
+    check!(tmpdir.write("b/target", foo));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("c/../../b", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Like `dotdot_even_more_in_middle_of_symlink`, but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_even_more_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir_all("b/c"));
+    check!(tmpdir.write("b/target", foo));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("c/../../b/.", &b, "up"));
+
+    let path = "b/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Same as `dotdot_even_more_in_middle_of_symlink`, but the symlink doesn't
+/// include `c`.
+#[test]
+fn dotdot_even_other_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir_all("b/c"));
+    check!(tmpdir.write("b/target", foo));
+    let c = check!(tmpdir.open_dir("b/c"));
+    check!(symlink_dir("../../b", &c, "up"));
+
+    let path = "b/c/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Like `dotdot_even_other_in_middle_of_symlink`, but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_even_other_in_middle_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir_all("b/c"));
+    check!(tmpdir.write("b/target", foo));
+    let c = check!(tmpdir.open_dir("b/c"));
+    check!(symlink_dir("../../b/.", &c, "up"));
+
+    let path = "b/c/up/target";
+    let mut file = check!(tmpdir.open(path));
+    let mut data = Vec::new();
+    check!(file.read_to_end(&mut data));
+    assert_eq!(data, foo);
+}
+
+/// Similar to `dotdot_in_middle_of_symlink`, but this time the symlink to
+/// `..` does happen to be the end of the path, so we need to make sure
+/// the implementation doesn't just do a stack pop when it sees the `..`
+/// leaving us with an `O_PATH` directory handle.
+#[test]
+fn dotdot_at_end_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir("b"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("..", &b, "up"));
+
+    // Do some things with `path` that might break with an `O_PATH` fd.
+    // On Linux, the `permissions` part doesn't because cap-std uses
+    // /proc/self/fd. But the `read_dir` part does.
+    let path = "b/up";
+
+    let perms = check!(tmpdir.metadata(path)).permissions();
+    check!(tmpdir.set_permissions(path, perms));
+
+    let contents = check!(tmpdir.read_dir(path));
+    for entry in contents {
+        let _entry = check!(entry);
+    }
+}
+
+/// Like `dotdot_at_end_of_symlink`, but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_at_end_of_symlink() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.write("target", foo));
+    check!(tmpdir.create_dir("b"));
+    let b = check!(tmpdir.open_dir("b"));
+    check!(symlink_dir("../.", &b, "up"));
+
+    // Do some things with `path` that might break with an `O_PATH` fd.
+    // On Linux, the `permissions` part doesn't because cap-std uses
+    // /proc/self/fd. But the `read_dir` part does.
+    let path = "b/up";
+
+    let perms = check!(tmpdir.metadata(path)).permissions();
+    check!(tmpdir.set_permissions(path, perms));
+
+    let contents = check!(tmpdir.read_dir(path));
+    for entry in contents {
+        let _entry = check!(entry);
+    }
+}
+
+/// Like `dotdot_at_end_of_symlink`, but do everything inside a new directory,
+/// so that `MaybeOwnedFile` doesn't reopen `.` which would artificially give
+/// us a non-`O_PATH` fd.
+#[test]
+fn dotdot_at_end_of_symlink_all_inside_dir() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir("dir"));
+    check!(tmpdir.write("dir/target", foo));
+    check!(tmpdir.create_dir("dir/b"));
+    let b = check!(tmpdir.open_dir("dir/b"));
+    check!(symlink_dir("..", &b, "up"));
+
+    // Do some things with `path` that might break with an `O_PATH` fd.
+    // On Linux, the `permissions` part doesn't because cap-std uses
+    // /proc/self/fd. But the `read_dir` part does.
+    let path = "dir/b/up";
+
+    let perms = check!(tmpdir.metadata(path)).permissions();
+    check!(tmpdir.set_permissions(path, perms));
+
+    let contents = check!(tmpdir.read_dir(path));
+    for entry in contents {
+        let _entry = check!(entry);
+    }
+}
+
+/// `dotdot_at_end_of_symlink_all_inside_dir`, but with a `/.` at the end.
+///
+/// Windows doesn't appear to like symlinks that end with `/.`.
+#[test]
+#[cfg_attr(windows, ignore)]
+fn dotdot_slashdot_at_end_of_symlink_all_inside_dir() {
+    let tmpdir = tmpdir();
+
+    let foo = b"foo";
+    check!(tmpdir.create_dir("dir"));
+    check!(tmpdir.write("dir/target", foo));
+    check!(tmpdir.create_dir("dir/b"));
+    let b = check!(tmpdir.open_dir("dir/b"));
+    check!(symlink_dir("../.", &b, "up"));
+
+    // Do some things with `path` that might break with an `O_PATH` fd.
+    // On Linux, the `permissions` part doesn't because cap-std uses
+    // /proc/self/fd. But the `read_dir` part does.
+    let path = "dir/b/up";
+
+    let perms = check!(tmpdir.metadata(path)).permissions();
+    check!(tmpdir.set_permissions(path, perms));
+
+    let contents = check!(tmpdir.read_dir(path));
+    for entry in contents {
+        let _entry = check!(entry);
+    }
+}