From 0bb83c5f470579893ed0f0a6f9686c4383c928ce Mon Sep 17 00:00:00 2001 From: Shlok Kulshreshtha Date: Mon, 17 Aug 2026 13:51:27 +0530 Subject: [PATCH] object-name: avoid use-after-free in get_oid_with_context_1() When a ":" argument names a relative path, resolve_relative_path() returns a newly allocated string and "cp" is pointed at it: new_path = resolve_relative_path(repo, cp); if (!new_path) { namelen = namelen - (cp - name); } else { cp = new_path; namelen = strlen(cp); } From there on "cp" and "new_path" name the same allocation. Later the memory location that "new_path" points to is freed. free(new_path); if (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp)) But here the reject_tree_in_index() passes "cp" to diagnose_invalid_index_path(), which calls strlen() on it, looks it up in the index, and formats it into its messages, allocating as it goes. All of this reads memory that has already been freed. Collapse the two exits into one to ensure a single free() that happens after the last use. Three things have to coincide to reach this: 1. The path has to be relative, or nothing is allocated and "cp" still points into the argument. 2. The entry found has to be a sparse directory, which needs a sparse index. 3. The argument has to get past the check in die_verify_filename() that skips a leading ':' followed by a non-alphanumeric, so ":0:./dir/" arrives here where ":./dir/" does not. Add a test to t1092 that covers the combination. It fails under SANITIZE=address without the change to object-name.c. This was reported in [1], and the shape used here was suggested in review [2], but that series was not rerolled and the fix never landed. [1] https://lore.kernel.org/git/cf6bcdb43e5b4abab464c30a914d64dc8e7a9925.1655336146.git.gitgitgadget@gmail.com/ [2] https://lore.kernel.org/git/xmqqy1xxw7rc.fsf@gitster.g/ Reported-by: Johannes Schindelin Original-patch-by: Johannes Schindelin Helped-by: Junio C Hamano Suggested-by: Patrick Steinhardt Signed-off-by: Shlok Kulshreshtha Signed-off-by: Junio C Hamano --- object-name.c | 14 ++++++++------ t/t1092-sparse-checkout-compatibility.sh | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/object-name.c b/object-name.c index 46159466ac..9221d532ff 100644 --- a/object-name.c +++ b/object-name.c @@ -1803,13 +1803,15 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo, memcmp(ce->name, cp, namelen)) break; if (ce_stage(ce) == stage) { + int ret = reject_tree_in_index(repo, only_to_die, ce, + stage, prefix, cp); + + if (!ret) { + oidcpy(oid, &ce->oid); + oc->mode = ce->ce_mode; + } free(new_path); - if (reject_tree_in_index(repo, only_to_die, ce, - stage, prefix, cp)) - return -1; - oidcpy(oid, &ce->oid); - oc->mode = ce->ce_mode; - return 0; + return ret; } pos++; } diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 8186da5c88..be85ee0c4b 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1357,6 +1357,17 @@ do " done +test_expect_success 'relative path to a sparse directory' ' + init_repos && + + # A "::" argument whose path is relative is resolved + # into a heap-allocated buffer, and a sparse directory found at that + # path is reported through it. Cover that combination, so that the + # reporting does not read the buffer after it has been released. + test_sparse_match test_must_fail git show :0:./folder1/ && + test_sparse_match test_must_fail git rev-parse :0:./folder1/ +' + test_expect_success 'submodule handling' ' init_repos &&