{"thread":{"id":"66252","subject":"[PATCH] history: do not dereference NULL when parent tree is missing","startedAt":"2026-09-02T12:07:56Z","lastAt":"2026-09-03T07:52:31Z","messageCount":4,"participants":["zkd18cjb@mail.ustc.edu.cn","Patrick Steinhardt","Jinbao Chen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"551747","messageId":"5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn","threadId":"66252","inReplyTo":null,"subject":"[PATCH] history: do not dereference NULL when parent tree is missing","fromName":"","fromEmail":"zkd18cjb@mail.ustc.edu.cn","sentAt":"2026-09-02T12:07:36Z","receivedAt":"2026-09-02T12:07:56Z","isPatch":true,"body":"write_ondisk_index() dereferences the return value of\nrepo_parse_tree_indirect() unconditionally.  If the parent commit's\ntree object is missing from the object store (corrupt repository,\nobject removed by tooling, or incomplete restore), the function\nreturns NULL and \"git history split\" crashes with a SIGSEGV\n(release build; UBSan reports a null-pointer member access at\nbuiltin/history.c:789).\n\nGuard the parse result and error out gracefully, following the\ncodebase convention for objects that cannot be loaded.\n\nSigned-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>\n---\nHi,\n\n(This was reported via the Git security contact, which suggested posting\nhere.  The security team asked me to post the fix on this list, as the\ncrash requires a missing object in a local repository and is not\nconsidered a security issue.)\n\n\"git history split\" crashes with a SIGSEGV when the commit's parent tree\nobject is missing from the object store (corrupt repository, object\nremoved by tooling, incomplete backup/mirror restore): write_ondisk_index()\ndereferences the NULL return value of repo_parse_tree_indirect().\nThe fix below guards the parse result, matching the codebase convention\nfor objects that cannot be loaded (\"if (!tree) return error(...)\").\n\nReproduction (verified on master @ f78ce2f7b6, x86-64 Linux):\n\n    git init r && cd r\n    git config user.email t@t && git config user.name t\n    echo a > f && git add f && git commit -qm one\n    echo b > f && git commit -qam two\n    tree=$(git rev-parse 'HEAD^^{tree}')\n    rm .git/objects/$(echo \"$tree\" | cut -c1-2)/$(echo \"$tree\" | cut -c3-)\n    GIT_EDITOR=true git history split HEAD\n\nBefore: release build SIGSEGV (exit 139, core dumped); UBSan reports\n\"member access within null pointer of type 'struct tree'\" at\nbuiltin/history.c:789.\nAfter: \"error: unable to parse tree <oid>\", exit 255, no crash.\nControl (tree object present) is unchanged.\n\n 1 file changed, 4 insertions(+)\n\ndiff --git a/builtin/history.c b/builtin/history.c\nindex 000155ad9c..097631f5ba 100644\n--- a/builtin/history.c\n+++ b/builtin/history.c\n@@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo,\n \topts.dst_index = &index;\n \n \ttree = repo_parse_tree_indirect(repo, oid);\n+\tif (!tree) {\n+\t\tret = error(_(\"unable to parse tree %s\"), oid_to_hex(oid));\n+\t\tgoto out;\n+\t}\n \tinit_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size);\n \n \tif (unpack_trees(1, &tree_desc, &opts)) {\n-- \n2.53.0\n\n"},{"id":"551828","messageId":"apkFBluOhc3SyKV1@pks.im","threadId":"66252","inReplyTo":"5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn","subject":"Re: [PATCH] history: do not dereference NULL when parent tree is missing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-03T05:26:30Z","receivedAt":"2026-09-03T05:26:44Z","isPatch":true,"body":"On Wed, Sep 02, 2026 at 08:07:36PM +0800, zkd18cjb@mail.ustc.edu.cn wrote:\n> write_ondisk_index() dereferences the return value of\n> repo_parse_tree_indirect() unconditionally.  If the parent commit's\n> tree object is missing from the object store (corrupt repository,\n> object removed by tooling, or incomplete restore), the function\n> returns NULL and \"git history split\" crashes with a SIGSEGV\n> (release build; UBSan reports a null-pointer member access at\n> builtin/history.c:789).\n\nNit: the information in the braces does not really add a lot of signal,\nI'd just drop it.\n\n> Guard the parse result and error out gracefully, following the\n> codebase convention for objects that cannot be loaded.\n> \n> Signed-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>\n\nNit: your From address does not match the Signed-off-by.\n\n[snip]\n> Reproduction (verified on master @ f78ce2f7b6, x86-64 Linux):\n> \n>     git init r && cd r\n>     git config user.email t@t && git config user.name t\n>     echo a > f && git add f && git commit -qm one\n>     echo b > f && git commit -qam two\n>     tree=$(git rev-parse 'HEAD^^{tree}')\n>     rm .git/objects/$(echo \"$tree\" | cut -c1-2)/$(echo \"$tree\" | cut -c3-)\n>     GIT_EDITOR=true git history split HEAD\n\nWe could of course add a test for this, but I don't really think that\nit's worth it.\n\n> diff --git a/builtin/history.c b/builtin/history.c\n> index 000155ad9c..097631f5ba 100644\n> --- a/builtin/history.c\n> +++ b/builtin/history.c\n> @@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo,\n>  \topts.dst_index = &index;\n>  \n>  \ttree = repo_parse_tree_indirect(repo, oid);\n> +\tif (!tree) {\n> +\t\tret = error(_(\"unable to parse tree %s\"), oid_to_hex(oid));\n> +\t\tgoto out;\n> +\t}\n\nYup, the fix looks obviously good to me, thanks!\n\nPatrick\n"},{"id":"551831","messageId":"20260903063657.2067303-1-zkd18cjb@mail.ustc.edu.cn","threadId":"66252","inReplyTo":"5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn","subject":"Re: [PATCH v2] history: do not dereference NULL when parent tree is missing","fromName":"Jinbao Chen","fromEmail":"zkd18cjb@mail.ustc.edu.cn","sentAt":"2026-09-03T06:36:57Z","receivedAt":"2026-09-03T06:37:14Z","isPatch":true,"body":"write_ondisk_index() dereferences the return value of\nrepo_parse_tree_indirect() unconditionally.  If the parent commit's\ntree object is missing from the object store (corrupt repository,\nobject removed by tooling, or incomplete restore), the function\nreturns NULL and \"git history split\" crashes with a SIGSEGV.\n\nGuard the parse result and error out gracefully, following the\ncodebase convention for objects that cannot be loaded.\n\nSigned-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>\n---\nThanks for the review!\n\nChanges since v1 (no functional changes):\n- Dropped the parenthetical note about the UBSan diagnostic from the\n  commit message, as suggested.\n- Sent with the From address matching the Signed-off-by.\n\n builtin/history.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/builtin/history.c b/builtin/history.c\nindex 000155ad9c..097631f5ba 100644\n--- a/builtin/history.c\n+++ b/builtin/history.c\n@@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo,\n \topts.dst_index = &index;\n \n \ttree = repo_parse_tree_indirect(repo, oid);\n+\tif (!tree) {\n+\t\tret = error(_(\"unable to parse tree %s\"), oid_to_hex(oid));\n+\t\tgoto out;\n+\t}\n \tinit_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size);\n \n \tif (unpack_trees(1, &tree_desc, &opts)) {\n-- \n2.53.0\n\n"},{"id":"551833","messageId":"apknMr9Jk-CzdLAR@pks.im","threadId":"66252","inReplyTo":"20260903063657.2067303-1-zkd18cjb@mail.ustc.edu.cn","subject":"Re: [PATCH v2] history: do not dereference NULL when parent tree is missing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-03T07:52:18Z","receivedAt":"2026-09-03T07:52:31Z","isPatch":true,"body":"On Thu, Sep 03, 2026 at 02:36:57PM +0800, Jinbao Chen wrote:\n> write_ondisk_index() dereferences the return value of\n> repo_parse_tree_indirect() unconditionally.  If the parent commit's\n> tree object is missing from the object store (corrupt repository,\n> object removed by tooling, or incomplete restore), the function\n> returns NULL and \"git history split\" crashes with a SIGSEGV.\n> \n> Guard the parse result and error out gracefully, following the\n> codebase convention for objects that cannot be loaded.\n> \n> Signed-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>\n> ---\n> Thanks for the review!\n> \n> Changes since v1 (no functional changes):\n> - Dropped the parenthetical note about the UBSan diagnostic from the\n>   commit message, as suggested.\n> - Sent with the From address matching the Signed-off-by.\n\nThanks, this version looks good to me!\n\nPatrick\n"}]}