{"thread":{"id":"66139","subject":"[PATCH] object-name: avoid use-after-free in get_oid_with_context_1()","startedAt":"2026-08-07T19:59:50Z","lastAt":"2026-08-17T15:44:00Z","messageCount":8,"participants":["Shlok Kulshreshtha","René Scharfe","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"550047","messageId":"20260807195941.99473-1-diy2903@gmail.com","threadId":"66139","inReplyTo":null,"subject":"[PATCH] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-08-07T19:59:40Z","receivedAt":"2026-08-07T19:59:50Z","isPatch":true,"body":"When a \":\" argument names a relative path, resolve_relative_path()\nreturns a newly allocated string and \"cp\" is pointed at it:\n\n\tnew_path = resolve_relative_path(repo, cp);\n\tif (!new_path) {\n\t\tnamelen = namelen - (cp - name);\n\t} else {\n\t\tcp = new_path;\n\t\tnamelen = strlen(cp);\n\t}\n\nFrom there on \"cp\" and \"new_path\" name the same allocation. Later the\nmemory location that \"new_path\" points to is freed.\n\n\tfree(new_path);\n\tif (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))\n\nBut here the reject_tree_in_index() passes \"cp\" to\ndiagnose_invalid_index_path(), which calls strlen() on it, looks it up\nin the index, and formats it into its messages, allocating as it goes.\nAll of this reads memory that has already been freed.\n\nCollapse the two exits into one to ensure a single free() that happens\nafter the last use.\n\nThree things have to coincide to reach this:\n\n1. The path has to be relative, or nothing is allocated and \"cp\"\nstill points into the argument.\n\n2. The entry found has to be a sparse\ndirectory, which needs a sparse index.\n\n3. The argument has to get past the check in die_verify_filename() that\nskips a leading ':' followed by a non-alphanumeric, so \":0:./dir/\"\narrives here where \":./dir/\" does not.\n\nAdd a test to t1092 that covers the combination. It fails under\nSANITIZE=address without the change to object-name.c.\n\nThis was reported in [1], and the shape used here was suggested in\nreview [2], but that series was not rerolled and the fix never landed.\n\n[1] https://lore.kernel.org/git/cf6bcdb43e5b4abab464c30a914d64dc8e7a9925.1655336146.git.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/xmqqy1xxw7rc.fsf@gitster.g/\n\nReported-by: Johannes Schindelin \nHelped-by: Junio C Hamano \nSigned-off-by: Shlok Kulshreshtha \n---\nThe three conditions make this awkward to reach by hand, so here is the\nrecipe:\n\n\tgit init sparse && cd sparse &&\n\tmkdir folder1 folder2 &&\n\techo a >folder1/a && echo b >folder2/b &&\n\tgit add -A && git commit -m init &&\n\tgit sparse-checkout init --cone --sparse-index &&\n\tgit sparse-checkout set folder1 &&\n\tgit show :0:./folder2/\n\nWithout the change below, no sanitizer is needed to see it. On 2.52.0\nthe buffer has already been reused by the time the message is formatted,\nso the path printed is whatever now sits in that memory, and it differs\nfrom run to run:\n\n\tfatal: path '' does not exist (neither on disk nor in the index)\n\tfatal: path 'M-6?:xM-@M-:M-L??X' does not exist (neither on disk nor in the index)\n\tfatal: path '?M-*JM-^M->M-YM-tn?H' does not exist (neither on disk nor in the index)\n\nStill without the change, built with SANITIZE=address, the same command\nreports\n\n\tERROR: AddressSanitizer: heap-use-after-free\n\tREAD of size 3 at 0x607000002a20\n\t #1 diagnose_invalid_index_path object-name.c:1653\n\t #2 get_oid_with_context_1 object-name.c:1807\n\t #3 maybe_die_on_misspelt_object_name\n\t #4 die_verify_filename setup.c:216\n\t #6 setup_revisions revision.c:3103\n\t #8 cmd_show log.c:694\n\tfreed by thread T0 here:\n\t #1 get_oid_with_context_1 object-name.c:1806\n\tpreviously allocated by thread T0 here:\n\t #5 prefix_path setup.c:149\n\t #6 get_oid_with_context_1 object-name.c:1784\n\nWith the change, the message reads \"folder2/\" every time and the\nsanitizer stays quiet.\n\n\"git diff\" and \"git rev-parse\" reach it the same way, and so does \"../\"\nfrom a subdirectory.\n\nThe new t1092 test fails without the object-name.c hunk and passes with\nit, under SANITIZE=address.\n object-name.c | 15 +++++++++------\n t/t1092-sparse-checkout-compatibility.sh | 11 +++++++++++\n 2 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..bffe795830 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1803,13 +1803,16 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\t memcmp(ce->name, cp, namelen))\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n+\t\t\t\tint ret = -1;\n+\n+\t\t\t\tif (!reject_tree_in_index(repo, only_to_die, ce,\n+\t\t\t\t\t\t\t stage, prefix, cp)) {\n+\t\t\t\t\toidcpy(oid, &ce->oid);\n+\t\t\t\t\toc->mode = ce->ce_mode;\n+\t\t\t\t\tret = 0;\n+\t\t\t\t}\n \t\t\t\tfree(new_path);\n-\t\t\t\tif (reject_tree_in_index(repo, only_to_die, ce,\n-\t\t\t\t\t\t\t stage, prefix, cp))\n-\t\t\t\t\treturn -1;\n-\t\t\t\toidcpy(oid, &ce->oid);\n-\t\t\t\toc->mode = ce->ce_mode;\n-\t\t\t\treturn 0;\n+\t\t\t\treturn ret;\n \t\t\t}\n \t\t\tpos++;\n \t\t}\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 4140c4d8ef..e88946c254 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1357,6 +1357,17 @@ do\n \t\"\n done\n \n+test_expect_success 'relative path to a sparse directory' '\n+\tinit_repos &&\n+\n+\t# A \"::\" argument whose path is relative is resolved\n+\t# into a heap-allocated buffer, and a sparse directory found at that\n+\t# path is reported through it. Cover that combination, so that the\n+\t# reporting does not read the buffer after it has been released.\n+\ttest_sparse_match test_must_fail git show :0:./folder1/ &&\n+\ttest_sparse_match test_must_fail git rev-parse :0:./folder1/\n+'\n+\n test_expect_success 'submodule handling' '\n \tinit_repos &&\n \n-- \n2.52.0\n\n"},{"id":"550098","messageId":"506880fc-5557-46a1-b26d-63349781849b@web.de","threadId":"66139","inReplyTo":"20260807195941.99473-1-diy2903@gmail.com","subject":"Re: [PATCH] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-08T16:23:33Z","receivedAt":"2026-08-08T16:23:44Z","isPatch":true,"body":"On 8/7/26 9:59 PM, Shlok Kulshreshtha wrote:\n> When a \":\" argument names a relative path, resolve_relative_path()\n> returns a newly allocated string and \"cp\" is pointed at it:\n> \n> \tnew_path = resolve_relative_path(repo, cp);\n> \tif (!new_path) {\n> \t\tnamelen = namelen - (cp - name);\n> \t} else {\n> \t\tcp = new_path;\n> \t\tnamelen = strlen(cp);\n> \t}\n> \n> From there on \"cp\" and \"new_path\" name the same allocation. Later the\n> memory location that \"new_path\" points to is freed.\n> \n> \tfree(new_path);\n> \tif (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))\n> \n> But here the reject_tree_in_index() passes \"cp\" to\n> diagnose_invalid_index_path(), which calls strlen() on it, looks it up\n> in the index, and formats it into its messages, allocating as it goes.\n> All of this reads memory that has already been freed.\n> \n> Collapse the two exits into one to ensure a single free() that happens\n> after the last use.\n> \n> Three things have to coincide to reach this:\n> \n> 1. The path has to be relative, or nothing is allocated and \"cp\"\n> still points into the argument.\n> \n> 2. The entry found has to be a sparse\n> directory, which needs a sparse index.\n> \n> 3. The argument has to get past the check in die_verify_filename() that\n> skips a leading ':' followed by a non-alphanumeric, so \":0:./dir/\"\n> arrives here where \":./dir/\" does not.\n> \n> Add a test to t1092 that covers the combination. It fails under\n> SANITIZE=address without the change to object-name.c.\n> \n> This was reported in [1], and the shape used here was suggested in\n> review [2], but that series was not rerolled and the fix never landed.\n> \n> [1] https://lore.kernel.org/git/cf6bcdb43e5b4abab464c30a914d64dc8e7a9925.1655336146.git.gitgitgadget@gmail.com/\n> [2] https://lore.kernel.org/git/xmqqy1xxw7rc.fsf@gitster.g/\n\nOh, from 2022, good find.\n\n> Reported-by: Johannes Schindelin \n\nOriginal-patch-by even, no?\n\n> Helped-by: Junio C Hamano \n> Signed-off-by: Shlok Kulshreshtha \n> ---\n> The three conditions make this awkward to reach by hand, so here is the\n> recipe:\n> \n> \tgit init sparse && cd sparse &&\n> \tmkdir folder1 folder2 &&\n> \techo a >folder1/a && echo b >folder2/b &&\n> \tgit add -A && git commit -m init &&\n> \tgit sparse-checkout init --cone --sparse-index &&\n> \tgit sparse-checkout set folder1 &&\n> \tgit show :0:./folder2/\n> \n> Without the change below, no sanitizer is needed to see it. On 2.52.0\n> the buffer has already been reused by the time the message is formatted,\n> so the path printed is whatever now sits in that memory, and it differs\n> from run to run:\n> \n> \tfatal: path '' does not exist (neither on disk nor in the index)\n> \tfatal: path 'M-6?:xM-@M-:M-L??X' does not exist (neither on disk nor in the index)\n> \tfatal: path '?M-*JM-^M->M-YM-tn?H' does not exist (neither on disk nor in the index)\n> \n> Still without the change, built with SANITIZE=address, the same command\n> reports\n> \n> \tERROR: AddressSanitizer: heap-use-after-free\n> \tREAD of size 3 at 0x607000002a20\n> \t #1 diagnose_invalid_index_path object-name.c:1653\n> \t #2 get_oid_with_context_1 object-name.c:1807\n> \t #3 maybe_die_on_misspelt_object_name\n> \t #4 die_verify_filename setup.c:216\n> \t #6 setup_revisions revision.c:3103\n> \t #8 cmd_show log.c:694\n> \tfreed by thread T0 here:\n> \t #1 get_oid_with_context_1 object-name.c:1806\n> \tpreviously allocated by thread T0 here:\n> \t #5 prefix_path setup.c:149\n> \t #6 get_oid_with_context_1 object-name.c:1784\n> \n> With the change, the message reads \"folder2/\" every time and the\n> sanitizer stays quiet.\n> \n> \"git diff\" and \"git rev-parse\" reach it the same way, and so does \"../\"\n> from a subdirectory.\n> \n> The new t1092 test fails without the object-name.c hunk and passes with\n> it, under SANITIZE=address.\n> object-name.c | 15 +++++++++------\n> t/t1092-sparse-checkout-compatibility.sh | 11 +++++++++++\n> 2 files changed, 20 insertions(+), 6 deletions(-)\n> \n> diff --git a/object-name.c b/object-name.c\n> index 83efba0ba6..bffe795830 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -1803,13 +1803,16 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n> \t\t\t memcmp(ce->name, cp, namelen))\n> \t\t\t\tbreak;\n> \t\t\tif (ce_stage(ce) == stage) {\n> +\t\t\t\tint ret = -1;\n> +\n> +\t\t\t\tif (!reject_tree_in_index(repo, only_to_die, ce,\n> +\t\t\t\t\t\t\t stage, prefix, cp)) {\n> +\t\t\t\t\toidcpy(oid, &ce->oid);\n> +\t\t\t\t\toc->mode = ce->ce_mode;\n> +\t\t\t\t\tret = 0;\n> +\t\t\t\t}\n> \t\t\t\tfree(new_path);\n> -\t\t\t\tif (reject_tree_in_index(repo, only_to_die, ce,\n> -\t\t\t\t\t\t\t stage, prefix, cp))\n> -\t\t\t\t\treturn -1;\n> -\t\t\t\toidcpy(oid, &ce->oid);\n> -\t\t\t\toc->mode = ce->ce_mode;\n> -\t\t\t\treturn 0;\n> +\t\t\t\treturn ret;\n\nOK\n\n> \t\t\t}\n> \t\t\tpos++;\n> \t\t}\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 4140c4d8ef..e88946c254 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -1357,6 +1357,17 @@ do\n> \t\"\n> done\n> \n> +test_expect_success 'relative path to a sparse directory' '\n> +\tinit_repos &&\n> +\n> +\t# A \"::\" argument whose path is relative is resolved\n> +\t# into a heap-allocated buffer, and a sparse directory found at that\n> +\t# path is reported through it. Cover that combination, so that the\n> +\t# reporting does not read the buffer after it has been released.\n> +\ttest_sparse_match test_must_fail git show :0:./folder1/ &&\n> +\ttest_sparse_match test_must_fail git rev-parse :0:./folder1/\n> +'\n> +\n> test_expect_success 'submodule handling' '\n> \tinit_repos &&\n> \n\nGood idea to add a test.\n\nRené\n\n"},{"id":"550111","messageId":"20260808200832.24313-1-diy2903@gmail.com","threadId":"66139","inReplyTo":"506880fc-5557-46a1-b26d-63349781849b@web.de","subject":"Re: [PATCH] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-08-08T20:08:28Z","receivedAt":"2026-08-08T20:08:40Z","isPatch":true,"body":"René Scharfe writes:\n\n> Original-patch-by even, no?\n\nYes, that's more accurate I guess. I'll update this in a v2.\n\nThanks for the review.\n\nShlok\n"},{"id":"550133","messageId":"20260809194212.77439-1-diy2903@gmail.com","threadId":"66139","inReplyTo":"20260808200832.24313-1-diy2903@gmail.com","subject":"[PATCH v2] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-08-09T19:42:09Z","receivedAt":"2026-08-09T19:42:21Z","isPatch":true,"body":"When a \":\" argument names a relative path, resolve_relative_path()\nreturns a newly allocated string and \"cp\" is pointed at it:\n\n\tnew_path = resolve_relative_path(repo, cp);\n\tif (!new_path) {\n\t\tnamelen = namelen - (cp - name);\n\t} else {\n\t\tcp = new_path;\n\t\tnamelen = strlen(cp);\n\t}\n\nFrom there on \"cp\" and \"new_path\" name the same allocation. Later the\nmemory location that \"new_path\" points to is freed.\n\n\tfree(new_path);\n\tif (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))\n\nBut here the reject_tree_in_index() passes \"cp\" to\ndiagnose_invalid_index_path(), which calls strlen() on it, looks it up\nin the index, and formats it into its messages, allocating as it goes.\nAll of this reads memory that has already been freed.\n\nCollapse the two exits into one to ensure a single free() that happens\nafter the last use.\n\nThree things have to coincide to reach this:\n\n1. The path has to be relative, or nothing is allocated and \"cp\"\nstill points into the argument.\n\n2. The entry found has to be a sparse\ndirectory, which needs a sparse index.\n\n3. The argument has to get past the check in die_verify_filename() that\nskips a leading ':' followed by a non-alphanumeric, so \":0:./dir/\"\narrives here where \":./dir/\" does not.\n\nAdd a test to t1092 that covers the combination. It fails under\nSANITIZE=address without the change to object-name.c.\n\nThis was reported in [1], and the shape used here was suggested in\nreview [2], but that series was not rerolled and the fix never landed.\n\n[1] https://lore.kernel.org/git/cf6bcdb43e5b4abab464c30a914d64dc8e7a9925.1655336146.git.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/xmqqy1xxw7rc.fsf@gitster.g/\n\nReported-by: Johannes Schindelin \nOriginal-patch-by: Johannes Schindelin \nHelped-by: Junio C Hamano \nSigned-off-by: Shlok Kulshreshtha \n---\nChanges since v1: credit Johannes as both, Reported-by and Original-patch-by,\nas pointed out by René. No change to the code or the test.\n\nThe three conditions make this awkward to reach by hand, so here is the\nrecipe:\n\n\tgit init sparse && cd sparse &&\n\tmkdir folder1 folder2 &&\n\techo a >folder1/a && echo b >folder2/b &&\n\tgit add -A && git commit -m init &&\n\tgit sparse-checkout init --cone --sparse-index &&\n\tgit sparse-checkout set folder1 &&\n\tgit show :0:./folder2/\n\nWithout the change below, no sanitizer is needed to see it. On 2.52.0\nthe buffer has already been reused by the time the message is formatted,\nso the path printed is whatever now sits in that memory, and it differs\nfrom run to run:\n\n\tfatal: path '' does not exist (neither on disk nor in the index)\n\tfatal: path 'M-6?:xM-@M-:M-L??X' does not exist (neither on disk nor in the index)\n\tfatal: path '?M-*JM-^M->M-YM-tn?H' does not exist (neither on disk nor in the index)\n\nStill without the change, built with SANITIZE=address, the same command\nreports\n\n\tERROR: AddressSanitizer: heap-use-after-free\n\tREAD of size 3 at 0x607000002a20\n\t #1 diagnose_invalid_index_path object-name.c:1653\n\t #2 get_oid_with_context_1 object-name.c:1807\n\t #3 maybe_die_on_misspelt_object_name\n\t #4 die_verify_filename setup.c:216\n\t #6 setup_revisions revision.c:3103\n\t #8 cmd_show log.c:694\n\tfreed by thread T0 here:\n\t #1 get_oid_with_context_1 object-name.c:1806\n\tpreviously allocated by thread T0 here:\n\t #5 prefix_path setup.c:149\n\t #6 get_oid_with_context_1 object-name.c:1784\n\nWith the change, the message reads \"folder2/\" every time and the\nsanitizer stays quiet.\n\n\"git diff\" and \"git rev-parse\" reach it the same way, and so does \"../\"\nfrom a subdirectory.\n\nThe new t1092 test fails without the object-name.c hunk and passes with\nit, under SANITIZE=address.\n object-name.c | 15 +++++++++------\n t/t1092-sparse-checkout-compatibility.sh | 11 +++++++++++\n 2 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..bffe795830 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1803,13 +1803,16 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\t memcmp(ce->name, cp, namelen))\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n+\t\t\t\tint ret = -1;\n+\n+\t\t\t\tif (!reject_tree_in_index(repo, only_to_die, ce,\n+\t\t\t\t\t\t\t stage, prefix, cp)) {\n+\t\t\t\t\toidcpy(oid, &ce->oid);\n+\t\t\t\t\toc->mode = ce->ce_mode;\n+\t\t\t\t\tret = 0;\n+\t\t\t\t}\n \t\t\t\tfree(new_path);\n-\t\t\t\tif (reject_tree_in_index(repo, only_to_die, ce,\n-\t\t\t\t\t\t\t stage, prefix, cp))\n-\t\t\t\t\treturn -1;\n-\t\t\t\toidcpy(oid, &ce->oid);\n-\t\t\t\toc->mode = ce->ce_mode;\n-\t\t\t\treturn 0;\n+\t\t\t\treturn ret;\n \t\t\t}\n \t\t\tpos++;\n \t\t}\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 4140c4d8ef..e88946c254 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1357,6 +1357,17 @@ do\n \t\"\n done\n \n+test_expect_success 'relative path to a sparse directory' '\n+\tinit_repos &&\n+\n+\t# A \"::\" argument whose path is relative is resolved\n+\t# into a heap-allocated buffer, and a sparse directory found at that\n+\t# path is reported through it. Cover that combination, so that the\n+\t# reporting does not read the buffer after it has been released.\n+\ttest_sparse_match test_must_fail git show :0:./folder1/ &&\n+\ttest_sparse_match test_must_fail git rev-parse :0:./folder1/\n+'\n+\n test_expect_success 'submodule handling' '\n \tinit_repos &&\n \n-- \n2.52.0\n\n"},{"id":"550158","messageId":"anltEAohp3F9Jbx5@pks.im","threadId":"66139","inReplyTo":"20260809194212.77439-1-diy2903@gmail.com","subject":"Re: [PATCH v2] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T06:17:52Z","receivedAt":"2026-08-10T06:18:00Z","isPatch":true,"body":"On Mon, Aug 10, 2026 at 01:12:09AM +0530, Shlok Kulshreshtha wrote:\n> diff --git a/object-name.c b/object-name.c\n> index 83efba0ba6..bffe795830 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -1803,13 +1803,16 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n> \t\t\t memcmp(ce->name, cp, namelen))\n> \t\t\t\tbreak;\n> \t\t\tif (ce_stage(ce) == stage) {\n> +\t\t\t\tint ret = -1;\n> +\n> +\t\t\t\tif (!reject_tree_in_index(repo, only_to_die, ce,\n> +\t\t\t\t\t\t\t stage, prefix, cp)) {\n> +\t\t\t\t\toidcpy(oid, &ce->oid);\n> +\t\t\t\t\toc->mode = ce->ce_mode;\n> +\t\t\t\t\tret = 0;\n> +\t\t\t\t}\n\nThe function only ever returns `-1` or `0` itself, so we could've\nwritten it this way:\n\n\n\tint ret = reject_tree_in_index(repo, only_to_die, ce,\n\t\t\t\t stage, prefix, cp);\n\tif (!ret) {\n\t\toidcpy(oid, &ce->oid);\n\t\toc->mode = ce->ce_mode;\n\t}\n\n\tfree(new_path);\n\treturn ret;\n\nBut I won't insist on that change, this is already a clear improvement.\n\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 4140c4d8ef..e88946c254 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -1357,6 +1357,17 @@ do\n> \t\"\n> done\n> \n> +test_expect_success 'relative path to a sparse directory' '\n> +\tinit_repos &&\n> +\n> +\t# A \"::\" argument whose path is relative is resolved\n> +\t# into a heap-allocated buffer, and a sparse directory found at that\n> +\t# path is reported through it. Cover that combination, so that the\n> +\t# reporting does not read the buffer after it has been released.\n> +\ttest_sparse_match test_must_fail git show :0:./folder1/ &&\n> +\ttest_sparse_match test_must_fail git rev-parse :0:./folder1/\n> +'\n\nYup, this test indeed catches the bug:\n\n --- sparse-checkout-err\t2026-08-10 06:13:59.698011294 +0000\n +++ sparse-index-err\t2026-08-10 06:13:59.703981906 +0000\n @@ -1 +1 @@\n -fatal: path 'folder1/' does not exist (neither on disk nor in the index)\n +fatal: path '�[UU?' does not exist (neither on disk nor in the index)\n\nOverall this looks good to me, thanks!\n\nPatrick\n"},{"id":"550694","messageId":"20260817082127.81132-1-diy2903@gmail.com","threadId":"66139","inReplyTo":"anltEAohp3F9Jbx5@pks.im","subject":"[PATCH v3] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-08-17T08:21:27Z","receivedAt":"2026-08-17T08:21:39Z","isPatch":true,"body":"When a \":\" argument names a relative path, resolve_relative_path()\nreturns a newly allocated string and \"cp\" is pointed at it:\n\n\tnew_path = resolve_relative_path(repo, cp);\n\tif (!new_path) {\n\t\tnamelen = namelen - (cp - name);\n\t} else {\n\t\tcp = new_path;\n\t\tnamelen = strlen(cp);\n\t}\n\nFrom there on \"cp\" and \"new_path\" name the same allocation. Later the\nmemory location that \"new_path\" points to is freed.\n\n\tfree(new_path);\n\tif (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))\n\nBut here the reject_tree_in_index() passes \"cp\" to\ndiagnose_invalid_index_path(), which calls strlen() on it, looks it up\nin the index, and formats it into its messages, allocating as it goes.\nAll of this reads memory that has already been freed.\n\nCollapse the two exits into one to ensure a single free() that happens\nafter the last use.\n\nThree things have to coincide to reach this:\n\n1. The path has to be relative, or nothing is allocated and \"cp\"\nstill points into the argument.\n\n2. The entry found has to be a sparse\ndirectory, which needs a sparse index.\n\n3. The argument has to get past the check in die_verify_filename() that\nskips a leading ':' followed by a non-alphanumeric, so \":0:./dir/\"\narrives here where \":./dir/\" does not.\n\nAdd a test to t1092 that covers the combination. It fails under\nSANITIZE=address without the change to object-name.c.\n\nThis was reported in [1], and the shape used here was suggested in\nreview [2], but that series was not rerolled and the fix never landed.\n\n[1] https://lore.kernel.org/git/cf6bcdb43e5b4abab464c30a914d64dc8e7a9925.1655336146.git.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/xmqqy1xxw7rc.fsf@gitster.g/\n\nReported-by: Johannes Schindelin \nOriginal-patch-by: Johannes Schindelin \nHelped-by: Junio C Hamano \nSuggested-by: Patrick Steinhardt \nSigned-off-by: Shlok Kulshreshtha \n---\nChanges since v2: apply Patrick's suggestion to assign\nreject_tree_in_index()'s return value directly, since it only\nreturns 0 or -1, which is exactly what this function needs to return\ntoo. No change to the test. This reduces redundancy.\n\nThe three conditions make this awkward to reach by hand, so here is the\nrecipe:\n\n\tgit init sparse && cd sparse &&\n\tmkdir folder1 folder2 &&\n\techo a >folder1/a && echo b >folder2/b &&\n\tgit add -A && git commit -m init &&\n\tgit sparse-checkout init --cone --sparse-index &&\n\tgit sparse-checkout set folder1 &&\n\tgit show :0:./folder2/\n\nWithout the change below, no sanitizer is needed to see it. On 2.52.0\nthe buffer has already been reused by the time the message is formatted,\nso the path printed is whatever now sits in that memory, and it differs\nfrom run to run:\n\n\tfatal: path '' does not exist (neither on disk nor in the index)\n\tfatal: path 'M-6?:xM-@M-:M-L??X' does not exist (neither on disk nor in the index)\n\tfatal: path '?M-*JM-^M->M-YM-tn?H' does not exist (neither on disk nor in the index)\n\nStill without the change, built with SANITIZE=address, the same command\nreports\n\n\tERROR: AddressSanitizer: heap-use-after-free\n\tREAD of size 3 at 0x607000002a20\n\t #1 diagnose_invalid_index_path object-name.c:1653\n\t #2 get_oid_with_context_1 object-name.c:1807\n\t #3 maybe_die_on_misspelt_object_name\n\t #4 die_verify_filename setup.c:216\n\t #6 setup_revisions revision.c:3103\n\t #8 cmd_show log.c:694\n\tfreed by thread T0 here:\n\t #1 get_oid_with_context_1 object-name.c:1806\n\tpreviously allocated by thread T0 here:\n\t #5 prefix_path setup.c:149\n\t #6 get_oid_with_context_1 object-name.c:1784\n\nWith the change, the message reads \"folder2/\" every time and the\nsanitizer stays quiet.\n\n\"git diff\" and \"git rev-parse\" reach it the same way, and so does \"../\"\nfrom a subdirectory.\n\nRe-verified against the current tree: the t1092 test still fails\nwithout the object-name.c change and passes with it, under\nSANITIZE=address.\n object-name.c | 14 ++++++++------\n t/t1092-sparse-checkout-compatibility.sh | 11 +++++++++++\n 2 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..026ff8c6dd 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1803,13 +1803,15 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\t memcmp(ce->name, cp, namelen))\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n+\t\t\t\tint ret = reject_tree_in_index(repo, only_to_die, ce,\n+\t\t\t\t\t\t\t stage, prefix, cp);\n+\n+\t\t\t\tif (!ret) {\n+\t\t\t\t\toidcpy(oid, &ce->oid);\n+\t\t\t\t\toc->mode = ce->ce_mode;\n+\t\t\t\t}\n \t\t\t\tfree(new_path);\n-\t\t\t\tif (reject_tree_in_index(repo, only_to_die, ce,\n-\t\t\t\t\t\t\t stage, prefix, cp))\n-\t\t\t\t\treturn -1;\n-\t\t\t\toidcpy(oid, &ce->oid);\n-\t\t\t\toc->mode = ce->ce_mode;\n-\t\t\t\treturn 0;\n+\t\t\t\treturn ret;\n \t\t\t}\n \t\t\tpos++;\n \t\t}\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 446c1776cb..05b54062b3 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1405,6 +1405,17 @@ do\n \t\"\n done\n \n+test_expect_success 'relative path to a sparse directory' '\n+\tinit_repos &&\n+\n+\t# A \"::\" argument whose path is relative is resolved\n+\t# into a heap-allocated buffer, and a sparse directory found at that\n+\t# path is reported through it. Cover that combination, so that the\n+\t# reporting does not read the buffer after it has been released.\n+\ttest_sparse_match test_must_fail git show :0:./folder1/ &&\n+\ttest_sparse_match test_must_fail git rev-parse :0:./folder1/\n+'\n+\n test_expect_success 'submodule handling' '\n \tinit_repos &&\n \n-- \n2.52.0\n\n"},{"id":"550696","messageId":"20260817083221.83212-1-diy2903@gmail.com","threadId":"66139","inReplyTo":"anltEAohp3F9Jbx5@pks.im","subject":"Re: [PATCH v2] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-08-17T08:32:21Z","receivedAt":"2026-08-17T08:32:27Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> But I won't insist on that change, this is already a clear improvement.\n\nThanks for the review. That does read cleaner, so I've included it in\nv3.\n\nShlok\n"},{"id":"550708","messageId":"xmqq1pbw7nwi.fsf@gitster.g","threadId":"66139","inReplyTo":"20260817082127.81132-1-diy2903@gmail.com","subject":"Re: [PATCH v3] object-name: avoid use-after-free in get_oid_with_context_1()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-17T15:43:57Z","receivedAt":"2026-08-17T15:44:00Z","isPatch":true,"body":"Shlok Kulshreshtha writes:\n\n> When a \":\" argument names a relative path, resolve_relative_path()\n> returns a newly allocated string and \"cp\" is pointed at it:\n>\n> \tnew_path = resolve_relative_path(repo, cp);\n> \tif (!new_path) {\n> \t\tnamelen = namelen - (cp - name);\n> \t} else {\n> \t\tcp = new_path;\n> \t\tnamelen = strlen(cp);\n> \t}\n>\n> From there on \"cp\" and \"new_path\" name the same allocation. Later the\n> memory location that \"new_path\" points to is freed.\n>\n> \tfree(new_path);\n> \tif (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))\n\nNicely described and ...\n\n> diff --git a/object-name.c b/object-name.c\n> index 83efba0ba6..026ff8c6dd 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -1803,13 +1803,15 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n> \t\t\t memcmp(ce->name, cp, namelen))\n> \t\t\t\tbreak;\n> \t\t\tif (ce_stage(ce) == stage) {\n> +\t\t\t\tint ret = reject_tree_in_index(repo, only_to_die, ce,\n> +\t\t\t\t\t\t\t stage, prefix, cp);\n> +\n> +\t\t\t\tif (!ret) {\n> +\t\t\t\t\toidcpy(oid, &ce->oid);\n> +\t\t\t\t\toc->mode = ce->ce_mode;\n> +\t\t\t\t}\n> \t\t\t\tfree(new_path);\n> -\t\t\t\tif (reject_tree_in_index(repo, only_to_die, ce,\n> -\t\t\t\t\t\t\t stage, prefix, cp))\n> -\t\t\t\t\treturn -1;\n> -\t\t\t\toidcpy(oid, &ce->oid);\n> -\t\t\t\toc->mode = ce->ce_mode;\n> -\t\t\t\treturn 0;\n> +\t\t\t\treturn ret;\n> \t\t\t}\n\n... the fix matches exactly what anybody would expect from the\nproblem description, i.e., \"Do not free new_path before we are done\nwith using cp\".\n\nWill queue. Thanks.\n"}]}