git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 3/3] unpack-trees: detect mismatching number of cache-tree/index entries

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 7, 2024, 04:38 UTC
Message-ID
<5e578c1f4161ba5c23fd5b27e792807b632e9c06.1728275640.git.ps@pks.im>
In-Reply-To
<cover.1728275640.git.ps@pks.im>

Same as the preceding commit, we unconditionally dereference the index's cache entries depending on the number of cache-tree entries, which can lead to a segfault when the cache-tree is corrupted. Fix this bug.

This also makes t4058 pass with the leak sanitizer enabled.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t4058-diff-duplicates.sh | 7 +++++--
 unpack-trees.c             | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh
index 3f602adb05..18e5ac88c3 100755
--- a/t/t4058-diff-duplicates.sh
+++ b/t/t4058-diff-duplicates.sh
@@ -10,6 +10,8 @@
 #   that the diff output isn't wildly unreasonable.
 
 test_description='test tree diff when trees have duplicate entries'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # make_tree_entry <mode> <mode> <sha1>
@@ -143,11 +145,12 @@ test_expect_success 'reset --hard does not segfault' '
 	test_grep "error: corrupted cache-tree has entries not present in index" err
 '
 
-test_expect_failure 'git diff HEAD does not segfault' '
+test_expect_success 'git diff HEAD does not segfault' '
 	git checkout base &&
 	GIT_TEST_CHECK_CACHE_TREE=false &&
 	git reset --hard &&
-	test_might_fail git diff HEAD
+	test_must_fail git diff HEAD 2>err &&
+	test_grep "error: corrupted cache-tree has entries not present in index" err
 '
 
 test_expect_failure 'can switch to another branch when status is empty' '
diff --git a/unpack-trees.c b/unpack-trees.c
index 21cc197d47..e10a9d1209 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -808,6 +808,8 @@ static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names,
 
 	if (!o->merge)
 		BUG("We need cache-tree to do this optimization");
+	if (nr_entries + pos > o->src_index->cache_nr)
+		return error(_("corrupted cache-tree has entries not present in index"));
 
 	/*
 	 * Do what unpack_callback() and unpack_single_entry() normally
-- 
2.47.0.rc0.dirty
Previous: Patrick Steinhardt
Message 13 of 13 in “cache-tree: fix segfaults with invalid cache-trees”
  1. 0/3 cache-tree: fix segfaults with invalid cache-treesPatrick Steinhardt, Sep 17, 2024
  2. 1/3 cache-tree: refactor verification to return error codesPatrick Steinhardt, Sep 17, 2024
  3. Eric SunshineSep 17, 2024
  4. Patrick SteinhardtSep 18, 2024
  5. 2/3 cache-tree: detect mismatching number of index entriesPatrick Steinhardt, Sep 17, 2024
  6. Junio C HamanoSep 19, 2024
  7. Patrick SteinhardtSep 24, 2024
  8. Junio C HamanoSep 24, 2024
  9. 3/3 unpack-trees: detect mismatching number of cache-tree/index entriesPatrick Steinhardt, Sep 17, 2024
  10. 0/3 cache-tree: fix segfaults with invalid cache-treesPatrick Steinhardt, Oct 7, 2024
  11. 1/3 cache-tree: refactor verification to return error codesPatrick Steinhardt, Oct 7, 2024
  12. 2/3 cache-tree: detect mismatching number of index entriesPatrick Steinhardt, Oct 7, 2024
  13. 3/3 unpack-trees: detect mismatching number of cache-tree/index entriesPatrick Steinhardt, Oct 7, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.