Skip to content

Commit 874cfd6

Browse files
committed
fix!: validate all components pushed onto the stack when creating leading paths.
This way, everyone using the stack with the purpose of altering the working tree will run additional checks to prevent callers from sneaking in forbidden paths. Note that these checks don't run otherwise, so one has to be careful to not forget to run these checks whenever needed.
1 parent eff4c00 commit 874cfd6

File tree

10 files changed

+94
-21
lines changed

10 files changed

+94
-21
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gix-fs/src/stack.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,12 @@ impl Stack {
6363
/// The path is also expected to be normalized, and should not contain extra separators, and must not contain `..`
6464
/// or have leading or trailing slashes (or additionally backslashes on Windows).
6565
pub fn make_relative_path_current(&mut self, relative: &Path, delegate: &mut dyn Delegate) -> std::io::Result<()> {
66-
if relative.as_os_str().is_empty() {
66+
if self.valid_components != 0 && relative.as_os_str().is_empty() {
6767
return Err(std::io::Error::new(
6868
std::io::ErrorKind::Other,
6969
"empty inputs are not allowed",
7070
));
7171
}
72-
// TODO: prevent leading or trailing slashes, on Windows also backslashes.
73-
// prevent leading backslashes on Windows as they are strange
7472
if self.valid_components == 0 {
7573
delegate.push_directory(self)?;
7674
}

gix-fs/tests/stack/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,20 @@ fn path_join_handling() {
157157
);
158158
}
159159

160+
#[test]
161+
fn empty_paths_are_noop_if_no_path_was_pushed_before() {
162+
let root = PathBuf::from(".");
163+
let mut s = Stack::new(root.clone());
164+
165+
let mut r = Record::default();
166+
s.make_relative_path_current("".as_ref(), &mut r).unwrap();
167+
assert_eq!(
168+
s.current_relative().to_string_lossy(),
169+
"",
170+
"it's fine to push an empty path to get a value for the stack root, once"
171+
);
172+
}
173+
160174
#[test]
161175
fn relative_components_are_invalid() {
162176
let root = PathBuf::from(".");

gix-worktree/Cargo.toml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ doctest = false
1616
[features]
1717
default = ["attributes"]
1818
## Instantiate stacks that can access `.gitattributes` information.
19-
attributes = ["dep:gix-attributes"]
19+
attributes = ["dep:gix-attributes", "dep:gix-validate"]
2020
## Data structures implement `serde::Serialize` and `serde::Deserialize`.
21-
serde = [ "dep:serde", "bstr/serde", "gix-index/serde", "gix-hash/serde", "gix-object/serde", "gix-attributes?/serde", "gix-ignore/serde" ]
21+
serde = ["dep:serde", "bstr/serde", "gix-index/serde", "gix-hash/serde", "gix-object/serde", "gix-attributes?/serde", "gix-ignore/serde"]
2222

2323
[dependencies]
2424
gix-index = { version = "^0.32.1", path = "../gix-index" }
@@ -28,10 +28,11 @@ gix-object = { version = "^0.42.0", path = "../gix-object" }
2828
gix-glob = { version = "^0.16.2", path = "../gix-glob" }
2929
gix-path = { version = "^0.10.7", path = "../gix-path" }
3030
gix-attributes = { version = "^0.22.2", path = "../gix-attributes", optional = true }
31+
gix-validate = { version = "^0.8.4", path = "../gix-validate", optional = true }
3132
gix-ignore = { version = "^0.11.2", path = "../gix-ignore" }
3233
gix-features = { version = "^0.38.1", path = "../gix-features" }
3334

34-
serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
35+
serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
3536
bstr = { version = "1.3.0", default-features = false }
3637

3738
document-features = { version = "0.2.0", optional = true }

gix-worktree/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ pub use gix_glob as glob;
1919
pub use gix_ignore as ignore;
2020
/// Provides types needed for using [`Stack::at_path()`] and [`Stack::at_entry()`].
2121
pub use gix_object as object;
22+
/// Provides types needed for using [`stack::State::for_checkout()`].
23+
#[cfg(feature = "attributes")]
24+
pub use gix_validate as validate;
2225

2326
/// A cache for efficiently executing operations on directories and files which are encountered in sorted order.
2427
/// That way, these operations can be re-used for subsequent invocations in the same directory.

gix-worktree/src/stack/delegate.rs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,18 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> {
8888
#[cfg(feature = "attributes")]
8989
State::CreateDirectoryAndAttributesStack {
9090
unlink_on_collision,
91+
validate,
9192
attributes: _,
92-
} => create_leading_directory(
93-
is_last_component,
94-
stack,
95-
self.is_dir,
96-
&mut self.statistics.delegate.num_mkdir_calls,
97-
*unlink_on_collision,
98-
)?,
93+
} => {
94+
validate_last_component(stack, *validate)?;
95+
create_leading_directory(
96+
is_last_component,
97+
stack,
98+
self.is_dir,
99+
&mut self.statistics.delegate.num_mkdir_calls,
100+
*unlink_on_collision,
101+
)?
102+
}
99103
#[cfg(feature = "attributes")]
100104
State::AttributesAndIgnoreStack { .. } | State::AttributesStack(_) => {}
101105
State::IgnoreStack(_) => {}
@@ -122,6 +126,29 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> {
122126
}
123127
}
124128

129+
#[cfg(feature = "attributes")]
130+
fn validate_last_component(stack: &gix_fs::Stack, opts: gix_validate::path::component::Options) -> std::io::Result<()> {
131+
// TODO: add mode-information
132+
let Some(last_component) = stack.current_relative().components().rev().next() else {
133+
return Ok(());
134+
};
135+
let last_component =
136+
gix_path::try_into_bstr(std::borrow::Cow::Borrowed(last_component.as_os_str().as_ref())).map_err(|_err| {
137+
std::io::Error::new(
138+
std::io::ErrorKind::Other,
139+
format!(
140+
"Path component {last_component:?} of path \"{}\" contained invalid UTF-8 and could not be validated",
141+
stack.current_relative().display()
142+
),
143+
)
144+
})?;
145+
146+
if let Err(err) = gix_validate::path::component(last_component.as_ref(), None, opts) {
147+
return Err(std::io::Error::new(std::io::ErrorKind::Other, err));
148+
}
149+
Ok(())
150+
}
151+
125152
#[cfg(feature = "attributes")]
126153
fn create_leading_directory(
127154
is_last_component: bool,

gix-worktree/src/stack/mod.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ pub enum State {
2828
CreateDirectoryAndAttributesStack {
2929
/// If there is a symlink or a file in our path, try to unlink it before creating the directory.
3030
unlink_on_collision: bool,
31+
/// Options to control how newly created path components should be validated.
32+
validate: gix_validate::path::component::Options,
3133
/// State to handle attribute information
3234
attributes: state::Attributes,
3335
},
@@ -135,18 +137,22 @@ impl Stack {
135137
/// All effects are similar to [`at_path()`][Self::at_path()].
136138
///
137139
/// If `relative` ends with `/` and `is_dir` is `None`, it is automatically assumed to be a directory.
138-
///
139-
/// ### Panics
140-
///
141-
/// on illformed UTF8 in `relative`
142140
pub fn at_entry<'r>(
143141
&mut self,
144142
relative: impl Into<&'r BStr>,
145143
is_dir: Option<bool>,
146144
objects: &dyn gix_object::Find,
147145
) -> std::io::Result<Platform<'_>> {
148146
let relative = relative.into();
149-
let relative_path = gix_path::from_bstr(relative);
147+
let relative_path = gix_path::try_from_bstr(relative).map_err(|_err| {
148+
std::io::Error::new(
149+
std::io::ErrorKind::Other,
150+
format!(
151+
"The path \"{}\" contained invalid UTF-8 and could not be turned into a path",
152+
relative
153+
),
154+
)
155+
})?;
150156

151157
self.at_path(
152158
relative_path,

gix-worktree/src/stack/state/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,14 @@ pub mod ignore;
6060
impl State {
6161
/// Configure a state to be suitable for checking out files, which only needs access to attribute files read from the index.
6262
#[cfg(feature = "attributes")]
63-
pub fn for_checkout(unlink_on_collision: bool, attributes: Attributes) -> Self {
63+
pub fn for_checkout(
64+
unlink_on_collision: bool,
65+
validate: gix_validate::path::component::Options,
66+
attributes: Attributes,
67+
) -> Self {
6468
State::CreateDirectoryAndAttributesStack {
6569
unlink_on_collision,
70+
validate,
6671
attributes,
6772
}
6873
}

gix-worktree/tests/worktree/stack/attributes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ fn baseline() -> crate::Result {
1717
let mut collection = gix_attributes::search::MetadataCollection::default();
1818
let state = gix_worktree::stack::State::for_checkout(
1919
false,
20+
Default::default(),
2021
state::Attributes::new(
2122
gix_attributes::Search::new_globals([base.join("user.attributes")], &mut buf, &mut collection)?,
2223
Some(git_dir.join("info").join("attributes")),

gix-worktree/tests/worktree/stack/create_directory.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ fn root_is_assumed_to_exist_and_files_in_root_do_not_create_directory() -> crate
88
let dir = tempdir()?;
99
let mut cache = Stack::new(
1010
dir.path().join("non-existing-root"),
11-
stack::State::for_checkout(false, Default::default()),
11+
stack::State::for_checkout(false, Default::default(), Default::default()),
1212
Default::default(),
1313
Vec::new(),
1414
Default::default(),
@@ -54,6 +54,23 @@ fn existing_directories_are_fine() -> crate::Result {
5454
Ok(())
5555
}
5656

57+
#[test]
58+
fn validation_to_each_component() -> crate::Result {
59+
let (mut cache, tmp) = new_cache();
60+
61+
let err = cache
62+
.at_path("valid/.gIt", Some(false), &gix_object::find::Never)
63+
.unwrap_err();
64+
assert_eq!(
65+
cache.statistics().delegate.num_mkdir_calls,
66+
1,
67+
"the valid directory was created"
68+
);
69+
assert!(tmp.path().join("valid").is_dir(), "it was actually created");
70+
assert_eq!(err.to_string(), "The .git name may never be used");
71+
Ok(())
72+
}
73+
5774
#[test]
5875
fn symlinks_or_files_in_path_are_forbidden_or_unlinked_when_forced() -> crate::Result {
5976
let (mut cache, tmp) = new_cache();
@@ -110,7 +127,7 @@ fn new_cache() -> (Stack, TempDir) {
110127
let dir = tempdir().unwrap();
111128
let cache = Stack::new(
112129
dir.path(),
113-
stack::State::for_checkout(false, Default::default()),
130+
stack::State::for_checkout(false, Default::default(), Default::default()),
114131
Default::default(),
115132
Vec::new(),
116133
Default::default(),

0 commit comments

Comments
 (0)