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

[PATCH 2/3] cache-tree: detect mismatching number of index entries

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 17, 2024, 07:13 UTC
Message-ID
<595693a6420b2571aabd51ed989bedfa0cfa62e2.1726556195.git.ps@pks.im>
In-Reply-To
<cover.1726556195.git.ps@pks.im>

In t4058 we have some tests that exercise git-read-tree(1) when used with a tree that contains duplicate entries. While the expectation is that we fail, we ideally should fail gracefully without a segfault.

But that is not the case: we never check that the number of entries in the cache-tree is less than or equal to the number of entries in the index. This can lead to an out-of-bounds read as we unconditionally access `istate->cache[idx]`, where `idx` is controlled by the number of cache-tree entries and the current position therein. The result is a segfault.

Fix this segfault by adding a sanity check for the number of index entries before dereferencing them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 cache-tree.c               |  5 +++++
 t/t4058-diff-duplicates.sh | 12 ++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 4228b6fad48..1e625673086 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -933,6 +933,11 @@ static int verify_one(struct repository *r,
 		pos = 0;
 	}
 
+	if (it->entry_count + pos > istate->cache_nr) {
+		ret = error(_("corrupted cache-tree has entries not present in index"));
+		goto out;
+	}
+
 	i = 0;
 	while (i < it->entry_count) {
 		struct cache_entry *ce = istate->cache[pos + i];
diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh
index 2501c89c1c9..3f602adb055 100755
--- a/t/t4058-diff-duplicates.sh
+++ b/t/t4058-diff-duplicates.sh
@@ -132,15 +132,15 @@ test_expect_success 'create a few commits' '
 	rm commit_id up final
 '
 
-test_expect_failure 'git read-tree does not segfault' '
-	test_when_finished rm .git/index.lock &&
-	test_might_fail git read-tree --reset base
+test_expect_success 'git read-tree does not segfault' '
+	test_must_fail git read-tree --reset base 2>err &&
+	test_grep "error: corrupted cache-tree has entries not present in index" err
 '
 
-test_expect_failure 'reset --hard does not segfault' '
-	test_when_finished rm .git/index.lock &&
+test_expect_success 'reset --hard does not segfault' '
 	git checkout base &&
-	test_might_fail git reset --hard
+	test_must_fail git reset --hard 2>err &&
+	test_grep "error: corrupted cache-tree has entries not present in index" err
 '
 
 test_expect_failure 'git diff HEAD does not segfault' '
-- 
2.46.0.551.gc5ee8f2d1c.dirty
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 5 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.