Skip to content

Commit c044919

Browse files
committed
add symlink checking for gix status
The motivation for this is git/git@f62ce3d .
1 parent a1794b5 commit c044919

File tree

7 files changed

+209
-8
lines changed

7 files changed

+209
-8
lines changed

gix-status/src/index_as_worktree/function.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ where
7272
State {
7373
buf: Vec::new(),
7474
odb_buf: Vec::new(),
75+
path_stack: crate::SymlinkCheck::new(worktree.to_owned()),
7576
timestamp,
7677
path_backing,
77-
worktree,
7878
options,
7979
},
8080
compare,
@@ -104,9 +104,8 @@ struct State<'a, 'b> {
104104
buf: Vec<u8>,
105105
odb_buf: Vec<u8>,
106106
timestamp: FileTime,
107-
// path_cache: fs::Cache TODO path cache
107+
path_stack: crate::SymlinkCheck,
108108
path_backing: &'b [u8],
109-
worktree: &'a Path,
110109
options: &'a Options,
111110
}
112111

@@ -195,12 +194,13 @@ impl<'index> State<'_, 'index> {
195194
E: std::error::Error + Send + Sync + 'static,
196195
Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E>,
197196
{
198-
// TODO fs cache
199197
let worktree_path = gix_path::try_from_bstr(git_path).map_err(|_| Error::IllformedUtf8)?;
200-
let worktree_path = self.worktree.join(worktree_path);
198+
let worktree_path = match self.path_stack.verified_path(worktree_path.as_ref()) {
199+
Ok(path) => path,
200+
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Some(Change::Removed)),
201+
Err(err) => return Err(Error::Io(err)),
202+
};
201203
let metadata = match worktree_path.symlink_metadata() {
202-
// TODO: check if any parent directory is a symlink
203-
// we need to use fs::Cache for that
204204
Ok(metadata) if metadata.is_dir() => {
205205
// index entries are normally only for files/symlinks
206206
// if a file turned into a directory it was removed
@@ -256,7 +256,7 @@ impl<'index> State<'_, 'index> {
256256

257257
let read_file = WorktreeBlob {
258258
buf: &mut self.buf,
259-
path: &worktree_path,
259+
path: worktree_path,
260260
entry,
261261
options: self.options,
262262
};

gix-status/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,13 @@ pub trait Pathspec {
3131
/// `is_dir` is `true` if `relative_path` is a directory.
3232
fn is_included(&mut self, relative_path: &BStr, is_dir: Option<bool>) -> bool;
3333
}
34+
35+
/// A stack that validates we are not going through a symlink in a way that is read-only.
36+
///
37+
/// It can efficiently validate paths when these are queried in sort-order, which leads to each component
38+
/// to only be checked once.
39+
pub struct SymlinkCheck {
40+
inner: gix_fs::Stack,
41+
}
42+
43+
mod stack;

gix-status/src/stack.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
use crate::SymlinkCheck;
2+
use gix_fs::Stack;
3+
use std::path::{Path, PathBuf};
4+
5+
impl SymlinkCheck {
6+
/// Create a new stack that starts operating at `root`.
7+
pub fn new(root: PathBuf) -> Self {
8+
Self {
9+
inner: gix_fs::Stack::new(root),
10+
}
11+
}
12+
13+
/// Return a valid filesystem path located in our root by appending `relative_path`, which is guaranteed to
14+
/// not pass through a symbolic link. That way the caller can be sure to not be misled by an attacker that
15+
/// tries to make us reach outside of the repository.
16+
///
17+
/// Note that the file pointed to by `relative_path` may still be a symbolic link, or not exist at all,
18+
/// and that an error may also be produced if directories on the path leading to the leaf
19+
/// component of `relative_path` are missing.
20+
///
21+
/// ### Note
22+
///
23+
/// On windows, no verification is performed, instead only the combined path is provided as usual.
24+
pub fn verified_path(&mut self, relative_path: &Path) -> std::io::Result<&Path> {
25+
self.inner.make_relative_path_current(relative_path, &mut Delegate)?;
26+
Ok(self.inner.current())
27+
}
28+
}
29+
30+
struct Delegate;
31+
32+
impl gix_fs::stack::Delegate for Delegate {
33+
fn push_directory(&mut self, _stack: &Stack) -> std::io::Result<()> {
34+
Ok(())
35+
}
36+
37+
fn push(&mut self, is_last_component: bool, stack: &Stack) -> std::io::Result<()> {
38+
#[cfg(windows)]
39+
{
40+
Ok(())
41+
}
42+
#[cfg(not(windows))]
43+
{
44+
if is_last_component {
45+
return Ok(());
46+
}
47+
48+
if stack.current().symlink_metadata()?.is_symlink() {
49+
return Err(std::io::Error::new(
50+
std::io::ErrorKind::Other,
51+
"Cannot step through symlink to perform an lstat",
52+
));
53+
}
54+
Ok(())
55+
}
56+
}
57+
58+
fn pop_directory(&mut self) {}
59+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
status_unchanged.tar.xz
22
status_changed.tar.xz
3+
symlink_stack.tar.xz
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/bash
2+
set -eu -o pipefail
3+
4+
mkdir base;
5+
(cd base
6+
touch file
7+
mkdir dir
8+
touch dir/file-in-dir
9+
(cd dir
10+
ln -s file-in-dir filelink
11+
mkdir subdir
12+
ln -s subdir dirlink
13+
)
14+
15+
ln -s file root-filelink
16+
ln -s dir root-dirlink
17+
)
18+
19+
ln -s base symlink-base
20+

gix-status/tests/stack/mod.rs

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
fn stack() -> gix_status::SymlinkCheck {
2+
stack_in("base")
3+
}
4+
5+
fn stack_in(dir: &str) -> gix_status::SymlinkCheck {
6+
gix_status::SymlinkCheck::new(
7+
gix_testtools::scripted_fixture_read_only_standalone("symlink_stack.sh")
8+
.expect("valid script")
9+
.join(dir),
10+
)
11+
}
12+
13+
#[test]
14+
fn paths_not_going_through_symlink_directories_are_ok_and_point_to_correct_item() -> crate::Result {
15+
for root in ["base", "symlink-base"] {
16+
let mut stack = stack_in(root);
17+
for (rela_path, expectation) in [
18+
("root-filelink", is_symlink as fn(&std::fs::Metadata) -> bool),
19+
("root-dirlink", is_symlinked_dir),
20+
("file", is_file),
21+
("dir/file-in-dir", is_file),
22+
("dir", is_dir),
23+
("dir/subdir", is_dir),
24+
("dir/filelink", is_symlink),
25+
("dir/dirlink", is_symlinked_dir),
26+
] {
27+
assert!(
28+
expectation(&stack.verified_path(rela_path.as_ref())?.symlink_metadata()?),
29+
"{rela_path:?} expectation failed"
30+
);
31+
}
32+
}
33+
Ok(())
34+
}
35+
36+
#[test]
37+
fn leaf_file_does_not_have_to_exist() -> crate::Result {
38+
assert!(!stack().verified_path("dir/does-not-exist".as_ref())?.exists());
39+
Ok(())
40+
}
41+
42+
#[test]
43+
#[cfg(not(windows))]
44+
fn intermediate_directories_have_to_exist_or_not_found_error() -> crate::Result {
45+
assert_eq!(
46+
stack()
47+
.verified_path("nonexisting-dir/file".as_ref())
48+
.unwrap_err()
49+
.kind(),
50+
std::io::ErrorKind::NotFound
51+
);
52+
Ok(())
53+
}
54+
55+
#[test]
56+
#[cfg(windows)]
57+
fn intermediate_directories_do_not_have_exist_for_success() -> crate::Result {
58+
assert!(stack().verified_path("nonexisting-dir/file".as_ref()).is_ok());
59+
Ok(())
60+
}
61+
62+
#[test]
63+
#[cfg_attr(
64+
windows,
65+
ignore = "on windows, symlinks appear to be files or dirs, is_symlink() doesn't work"
66+
)]
67+
fn paths_leading_through_symlinks_are_rejected() {
68+
let mut stack = stack();
69+
assert_eq!(
70+
stack
71+
.verified_path("root-dirlink/file-in-dir".as_ref())
72+
.unwrap_err()
73+
.kind(),
74+
std::io::ErrorKind::Other,
75+
"root-dirlink is a symlink to a directory"
76+
);
77+
78+
assert_eq!(
79+
stack.verified_path("dir/dirlink/nothing".as_ref()).unwrap_err().kind(),
80+
std::io::ErrorKind::Other,
81+
"root-dirlink is a symlink to a directory"
82+
);
83+
}
84+
85+
fn is_symlink(m: &std::fs::Metadata) -> bool {
86+
if cfg!(windows) {
87+
// On windows, symlinks can't be seen, at least not through std
88+
m.is_file()
89+
} else {
90+
m.is_symlink()
91+
}
92+
}
93+
94+
fn is_symlinked_dir(m: &std::fs::Metadata) -> bool {
95+
if cfg!(windows) {
96+
// On windows, symlinks can't be seen, at least not through std
97+
m.is_dir()
98+
} else {
99+
m.is_symlink()
100+
}
101+
}
102+
fn is_file(m: &std::fs::Metadata) -> bool {
103+
m.is_file()
104+
}
105+
fn is_dir(m: &std::fs::Metadata) -> bool {
106+
m.is_dir()
107+
}

gix-status/tests/worktree.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
1+
pub use gix_testtools::Result;
2+
13
mod status;
24
use status::*;
5+
6+
mod stack;

0 commit comments

Comments
 (0)