common_prefix_len() derives the common prefix solely from positive pathspecs, skipping those marked with PATHSPEC_EXCLUDE. However, match_pathspec_with_flags() also passes that prefix when matching the negative pathspecs.
A negative pathspec may be shorter than the prefix. In that case, match_pathspec_item() advances item->match beyond its allocation and subtracts the prefix from item->len, producing a negative matchlen. It then dereferences the out-of-bounds pointer. If the resulting byte is not NUL, matchlen is converted to size_t when passed to ps_strncmp(), which may cause a much larger out-of-bounds read.
The problem can be reproduced with AddressSanitizer:
make SANITIZE=address CFLAGS="-g -O0" git
git init test &&
cd test &&
DIR=$(printf "a%.0s" {1..150}) &&
mkdir -p "$DIR" &&
touch "$DIR/f.txt" &&
git add -A &&
git commit -m test &&
../git ls-files -- "$DIR/" ":(exclude)xy"This reports a heap-buffer-overflow. Without AddressSanitizer, the output may depend on the contents of memory following the negative pathspec.
Fix the bug by using a zero prefix when matching negative pathspecs. Add a regression test that combines a positive pathspec with a longer common prefix and a shorter, unrelated negative pathspec.
Signed-off-by: Yannik Tausch <dev@ytausch.de> ---
Note that I already sent information about this issue to the git security mailing list on July 31, where I was informed that a fix of this kind does not require an embargo.
dir.c | 2 +- t/t6132-pathspec-exclude.sh | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
Show changes to 2 files +10 −1
dir.c, t/t6132-pathspec-exclude.sh
diff --git a/dir.c b/dir.c index 95d8a1cce9..7072715389 100644 --- a/dir.c +++ b/dir.c @@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate, if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; negative = do_match_pathspec(istate, ps, name, namelen, - prefix, seen, + 0, seen, flags | DO_MATCH_EXCLUDE); return negative ? 0 : positive; } diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh index 9fdafeb1e9..ad919cc739 100755 --- a/t/t6132-pathspec-exclude.sh +++ b/t/t6132-pathspec-exclude.sh @@ -183,6 +183,15 @@ EOF test_cmp expect actual ' +test_expect_success 'negative pathspec shorter than positive pathspec prefix' ' + git ls-files -- sub/sub/ ":(exclude)sub2" >actual && + cat <<-\EOF >expect && + sub/sub/file + sub/sub/sub/file + EOF + test_cmp expect actual +' + test_expect_success 'multiple exclusions' ' git ls-files -- ":^*/file2" ":^sub2" >actual && cat <<-\EOF >expect &&