Re: [PATCH v2] object-name: avoid use-after-free in get_oid_with_context_1()
Patrick Steinhardt <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Aug 10, 2026 at 01:12:09AM +0530, Shlok Kulshreshtha wrote:
> diff --git a/object-name.c b/object-name.c
> index 83efba0ba6..bffe795830 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1803,13 +1803,16 @@ 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 = -1;
> +
> + if (!reject_tree_in_index(repo, only_to_die, ce,
> + stage, prefix, cp)) {
> + oidcpy(oid, &ce->oid);
> + oc->mode = ce->ce_mode;
> + ret = 0;
> + }
The function only ever returns `-1` or `0` itself, so we could've
written it this way:
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);
return ret;
But I won't insist on that change, this is already a clear improvement.
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 4140c4d8ef..e88946c254 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 ":<stage>:<path>" 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/
> +'
Yup, this test indeed catches the bug:
--- sparse-checkout-err 2026-08-10 06:13:59.698011294 +0000
+++ sparse-index-err 2026-08-10 06:13:59.703981906 +0000
@@ -1 +1 @@
-fatal: path 'folder1/' does not exist (neither on disk nor in the index)
+fatal: path '�[UU?' does not exist (neither on disk nor in the index)
Overall this looks good to me, thanks!
Patrick