diff --git a/cap-primitives/src/fs/read_link.rs b/cap-primitives/src/fs/read_link.rs index f1e91ef8..db682c16 100644 --- a/cap-primitives/src/fs/read_link.rs +++ b/cap-primitives/src/fs/read_link.rs @@ -1,7 +1,7 @@ //! This defines `read_link`, the primary entrypoint to sandboxed symbolic link //! dereferencing. -use crate::fs::{errors, read_link_impl}; +use crate::fs::read_link_impl; #[cfg(racy_asserts)] use crate::fs::{map_result, read_link_unchecked, stat, FollowSymlinks}; use std::path::{Path, PathBuf}; @@ -24,27 +24,12 @@ pub fn read_link_contents(start: &fs::File, path: &Path) -> io::Result result } -/// Perform a `readlinkat`-like operation, ensuring that the resolution of the -/// path never escapes the directory tree rooted at `start`, and also verifies -/// that the link target is not absolute. +/// Perform a `readlinkat`-like operation. #[cfg_attr(not(racy_asserts), allow(clippy::let_and_return))] #[inline] pub fn read_link(start: &fs::File, path: &Path) -> io::Result { // Call the underlying implementation. - let result = read_link_contents(start, path); - - // Don't allow reading symlinks to absolute paths. This isn't strictly - // necessary to preserve the sandbox, since `open` will refuse to follow - // absolute paths in any case. However, it is useful to enforce this - // restriction to avoid leaking information about the host filesystem - // outside the sandbox. - if let Ok(path) = &result { - if path.has_root() { - return Err(errors::escape_attempt()); - } - } - - result + read_link_contents(start, path) } #[cfg(racy_asserts)] diff --git a/tests/fs.rs b/tests/fs.rs index a8b90c24..b803dfba 100644 --- a/tests/fs.rs +++ b/tests/fs.rs @@ -12,8 +12,7 @@ use std::io::prelude::*; #[cfg(target_os = "macos")] use crate::sys::weak::weak; -use cap_std::ambient_authority; -use cap_std::fs::{self, Dir, OpenOptions}; +use cap_std::fs::{self, OpenOptions}; #[cfg(target_os = "macos")] use libc::{c_char, c_int}; use std::io::{self, ErrorKind, SeekFrom}; @@ -935,24 +934,6 @@ fn symlink_noexist() { #[test] fn read_link() { - if cfg!(windows) { - // directory symlink - let root = Dir::open_ambient_dir(r"C:\", ambient_authority()).unwrap(); - error_contains!( - root.read_link(r"Users\All Users"), - "a path led outside of the filesystem" - ); - // junction - error_contains!( - root.read_link(r"Users\Default User"), - "a path led outside of the filesystem" - ); - // junction with special permissions - error_contains!( - root.read_link(r"Documents and Settings\"), - "a path led outside of the filesystem" - ); - } let tmpdir = tmpdir(); let link = "link"; if !got_symlink_permission(&tmpdir) { diff --git a/tests/fs_utf8.rs b/tests/fs_utf8.rs index 8ed98eb8..3e8d9ac4 100644 --- a/tests/fs_utf8.rs +++ b/tests/fs_utf8.rs @@ -14,8 +14,7 @@ use std::io::prelude::*; #[cfg(target_os = "macos")] use crate::sys::weak::weak; use camino::{Utf8Path as Path, Utf8PathBuf as PathBuf}; -use cap_std::ambient_authority; -use cap_std::fs_utf8::{self as fs, Dir, OpenOptions}; +use cap_std::fs_utf8::{self as fs, OpenOptions}; #[cfg(target_os = "macos")] use libc::{c_char, c_int}; use std::io::{self, ErrorKind, SeekFrom}; @@ -938,24 +937,6 @@ fn symlink_noexist() { #[test] fn read_link() { - if cfg!(windows) { - // directory symlink - let root = Dir::open_ambient_dir(r"C:\", ambient_authority()).unwrap(); - error_contains!( - root.read_link(r"Users\All Users"), - "a path led outside of the filesystem" - ); - // junction - error_contains!( - root.read_link(r"Users\Default User"), - "a path led outside of the filesystem" - ); - // junction with special permissions - error_contains!( - root.read_link(r"Documents and Settings\"), - "a path led outside of the filesystem" - ); - } let tmpdir = tmpdir(); let link = "link"; if !got_symlink_permission(&tmpdir) { diff --git a/tests/symlinks.rs b/tests/symlinks.rs index 5323a881..d283d481 100644 --- a/tests/symlinks.rs +++ b/tests/symlinks.rs @@ -4,6 +4,7 @@ mod sys_common; use cap_fs_ext::DirExt; use cap_std::ambient_authority; use cap_std::fs::Dir; +use std::path::Path; use sys_common::io::tmpdir; use sys_common::symlink_supported; @@ -116,19 +117,19 @@ fn readlink_absolute() { let tmpdir = check!(Dir::open_ambient_dir(dir.path(), ambient_authority())); #[cfg(not(windows))] - error_contains!( - tmpdir.read_link("thing_symlink"), - "a path led outside of the filesystem" + assert_eq!( + tmpdir.read_link("thing_symlink").unwrap(), + Path::new("/thing") ); #[cfg(windows)] - error_contains!( - tmpdir.read_link("file_symlink_file"), - "a path led outside of the filesystem" + assert_eq!( + tmpdir.read_link("file_symlink_file").unwrap(), + Path::new("/file") ); #[cfg(windows)] - error_contains!( - tmpdir.read_link("dir_symlink_dir"), - "a path led outside of the filesystem" + assert_eq!( + tmpdir.read_link("dir_symlink_dir").unwrap(), + Path::new("/dir") ); }