From 3340b14ddca0a9e32e259e54ce71132c9e371506 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 16 May 2024 13:52:41 -0600 Subject: [PATCH] Add Dir::{read_link_contents,symlink_contents} methods (#356) These allow creating and readling symlinks whose target may be an absolute path. There is no security risk because security is as always provided by the OS; the OS's openat implementation will refuse to follow a symlink whose target is an absolute path. Fixes #354 --- cap-std/src/fs/dir.rs | 48 ++++++++++++++++++++++++++++++++++---- cap-std/src/fs_utf8/dir.rs | 44 +++++++++++++++++++++++++++++++++- tests/fs.rs | 38 ++++++++++++++++++++++++++++++ tests/fs_utf8.rs | 32 +++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 5 deletions(-) diff --git a/cap-std/src/fs/dir.rs b/cap-std/src/fs/dir.rs index 92ce1f5c..74268389 100644 --- a/cap-std/src/fs/dir.rs +++ b/cap-std/src/fs/dir.rs @@ -7,8 +7,9 @@ use crate::os::unix::net::{UnixDatagram, UnixListener, UnixStream}; use cap_primitives::fs::set_permissions; use cap_primitives::fs::{ canonicalize, copy, create_dir, hard_link, open, open_ambient_dir, open_dir, open_parent_dir, - read_base_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, remove_open_dir, - remove_open_dir_all, rename, stat, DirOptions, FollowSymlinks, Permissions, + read_base_dir, read_dir, read_link, read_link_contents, remove_dir, remove_dir_all, + remove_file, remove_open_dir, remove_open_dir_all, rename, stat, DirOptions, FollowSymlinks, + Permissions, }; use cap_primitives::AmbientAuthority; use io_lifetimes::AsFilelike; @@ -21,7 +22,7 @@ use std::path::{Path, PathBuf}; use std::{fmt, fs}; #[cfg(not(windows))] use { - cap_primitives::fs::symlink, + cap_primitives::fs::{symlink, symlink_contents}, io_extras::os::rustix::{AsRawFd, FromRawFd, IntoRawFd, RawFd}, }; #[cfg(windows)] @@ -284,12 +285,22 @@ impl Dir { /// Reads a symbolic link, returning the file that the link points to. /// /// This corresponds to [`std::fs::read_link`], but only accesses paths - /// relative to `self`. + /// relative to `self`. Unlike [`read_link_contents`], this method considers it an error if + /// the link's target is an absolute path. #[inline] pub fn read_link>(&self, path: P) -> io::Result { read_link(&self.std_file, path.as_ref()) } + /// Reads a symbolic link, returning the file that the link points to. + /// + /// This corresponds to [`std::fs::read_link`]. but only accesses paths + /// relative to `self`. + #[inline] + pub fn read_link_contents>(&self, path: P) -> io::Result { + read_link_contents(&self.std_file, path.as_ref()) + } + /// Read the entire contents of a file into a string. /// /// This corresponds to [`std::fs::read_to_string`], but only accesses @@ -411,6 +422,9 @@ impl Dir { /// This corresponds to [`std::os::unix::fs::symlink`], but only accesses /// paths relative to `self`. /// + /// Unlike [`symlink_contents`] this method will return an error if `original` is an absolute + /// path. + /// /// [`std::os::unix::fs::symlink`]: https://doc.rust-lang.org/std/os/unix/fs/fn.symlink.html #[cfg(not(windows))] #[inline] @@ -418,6 +432,32 @@ impl Dir { symlink(original.as_ref(), &self.std_file, link.as_ref()) } + /// Creates a new symbolic link on a filesystem. + /// + /// The `original` argument provides the target of the symlink. The `link` + /// argument provides the name of the created symlink. + /// + /// Despite the argument ordering, `original` is not resolved relative to + /// `self` here. `link` is resolved relative to `self`, and `original` is + /// not resolved within this function. + /// + /// The `link` path is resolved when the symlink is dereferenced, relative + /// to the directory that contains it. + /// + /// This corresponds to [`std::os::unix::fs::symlink`], but only accesses + /// paths relative to `self`. + /// + /// [`std::os::unix::fs::symlink`]: https://doc.rust-lang.org/std/os/unix/fs/fn.symlink.html + #[cfg(not(windows))] + #[inline] + pub fn symlink_contents, Q: AsRef>( + &self, + original: P, + link: Q, + ) -> io::Result<()> { + symlink_contents(original.as_ref(), &self.std_file, link.as_ref()) + } + /// Creates a new file symbolic link on a filesystem. /// /// The `original` argument provides the target of the symlink. The `link` diff --git a/cap-std/src/fs_utf8/dir.rs b/cap-std/src/fs_utf8/dir.rs index f3199cdb..3f0c5f2b 100644 --- a/cap-std/src/fs_utf8/dir.rs +++ b/cap-std/src/fs_utf8/dir.rs @@ -230,13 +230,24 @@ impl Dir { /// Reads a symbolic link, returning the file that the link points to. /// /// This corresponds to [`std::fs::read_link`], but only accesses paths - /// relative to `self`. + /// relative to `self`. Unlike [`read_link_contents`], this method considers it an error if + /// the link's target is an absolute path. #[inline] pub fn read_link>(&self, path: P) -> io::Result { let path = from_utf8(path.as_ref())?; self.cap_std.read_link(path).and_then(to_utf8) } + /// Reads a symbolic link, returning the file that the link points to. + /// + /// This corresponds to [`std::fs::read_link`]. but only accesses paths + /// relative to `self`. + #[inline] + pub fn read_link_contents>(&self, path: P) -> io::Result { + let path = from_utf8(path.as_ref())?; + self.cap_std.read_link_contents(path).and_then(to_utf8) + } + /// Read the entire contents of a file into a string. /// /// This corresponds to [`std::fs::read_to_string`], but only accesses @@ -371,6 +382,9 @@ impl Dir { /// This corresponds to [`std::os::unix::fs::symlink`], but only accesses /// paths relative to `self`. /// + /// Unlike [`symlink_contents`] this method will return an error if `original` is an absolute + /// path. + /// /// [`std::os::unix::fs::symlink`]: https://doc.rust-lang.org/std/os/unix/fs/fn.symlink.html #[cfg(not(windows))] #[inline] @@ -384,6 +398,34 @@ impl Dir { self.cap_std.symlink(original, link) } + /// Creates a new symbolic link on a filesystem. + /// + /// The `original` argument provides the target of the symlink. The `link` + /// argument provides the name of the created symlink. + /// + /// Despite the argument ordering, `original` is not resolved relative to + /// `self` here. `link` is resolved relative to `self`, and `original` is + /// not resolved within this function. + /// + /// The `link` path is resolved when the symlink is dereferenced, relative + /// to the directory that contains it. + /// + /// This corresponds to [`std::os::unix::fs::symlink`], but only accesses + /// paths relative to `self`. + /// + /// [`std::os::unix::fs::symlink`]: https://doc.rust-lang.org/std/os/unix/fs/fn.symlink.html + #[cfg(not(windows))] + #[inline] + pub fn symlink_contents, Q: AsRef>( + &self, + original: P, + link: Q, + ) -> io::Result<()> { + let original = from_utf8(original.as_ref())?; + let link = from_utf8(link.as_ref())?; + self.cap_std.symlink_contents(original, link) + } + /// Creates a new file symbolic link on a filesystem. /// /// The `original` argument provides the target of the symlink. The `link` diff --git a/tests/fs.rs b/tests/fs.rs index a8b90c24..76a37640 100644 --- a/tests/fs.rs +++ b/tests/fs.rs @@ -29,6 +29,14 @@ use sys_common::symlink_junction; use rand::rngs::StdRng; use rand::{RngCore, SeedableRng}; +#[cfg(not(windows))] +fn symlink_contents, Q: AsRef>( + src: P, + tmpdir: &TempDir, + dst: Q, +) -> io::Result<()> { + tmpdir.symlink_contents(src, dst) +} #[cfg(not(windows))] fn symlink_dir, Q: AsRef>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> { tmpdir.symlink(src, dst) @@ -971,6 +979,36 @@ fn readlink_not_symlink() { } } +#[cfg(not(windows))] +#[test] +fn read_link_contents() { + let tmpdir = tmpdir(); + let link = "link"; + if !got_symlink_permission(&tmpdir) { + return; + }; + check!(symlink_file(&"foo", &tmpdir, &link)); + assert_eq!( + check!(tmpdir.read_link_contents(&link)).to_str().unwrap(), + "foo" + ); +} + +#[cfg(not(windows))] +#[test] +fn read_link_contents_absolute() { + let tmpdir = tmpdir(); + let link = "link"; + if !got_symlink_permission(&tmpdir) { + return; + }; + check!(symlink_contents(&"/foo", &tmpdir, &link)); + assert_eq!( + check!(tmpdir.read_link_contents(&link)).to_str().unwrap(), + "/foo" + ); +} + #[test] fn links_work() { let tmpdir = tmpdir(); diff --git a/tests/fs_utf8.rs b/tests/fs_utf8.rs index 8ed98eb8..eaa72fb0 100644 --- a/tests/fs_utf8.rs +++ b/tests/fs_utf8.rs @@ -30,6 +30,14 @@ use sys_common::symlink_junction_utf8 as symlink_junction; use rand::rngs::StdRng; use rand::{RngCore, SeedableRng}; +#[cfg(not(windows))] +fn symlink_contents, Q: AsRef>( + src: P, + tmpdir: &TempDir, + dst: Q, +) -> io::Result<()> { + tmpdir.symlink_contents(src, dst) +} #[cfg(not(windows))] fn symlink_dir, Q: AsRef>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> { tmpdir.symlink(src, dst) @@ -974,6 +982,30 @@ fn readlink_not_symlink() { } } +#[cfg(not(windows))] +#[test] +fn read_link_contents() { + let tmpdir = tmpdir(); + let link = "link"; + if !got_symlink_permission(&tmpdir) { + return; + }; + check!(symlink_file(&"foo", &tmpdir, &link)); + assert_eq!(check!(tmpdir.read_link_contents(&link)).as_str(), "foo"); +} + +#[cfg(not(windows))] +#[test] +fn read_link_contents_absolute() { + let tmpdir = tmpdir(); + let link = "link"; + if !got_symlink_permission(&tmpdir) { + return; + }; + check!(symlink_contents(&"/foo", &tmpdir, &link)); + assert_eq!(check!(tmpdir.read_link_contents(&link)).as_str(), "/foo"); +} + #[test] fn links_work() { let tmpdir = tmpdir();