# [PATCH] history: do not dereference NULL when parent tree is missing 4 messages from 2026-09-02 to 2026-09-03. Participants: zkd18cjb@mail.ustc.edu.cn, Patrick Steinhardt, Jinbao Chen. Thread: https://gitlist.dev/t/66252 ## zkd18cjb@mail.ustc.edu.cn, 2026-09-02 12:07 Subject: [PATCH] history: do not dereference NULL when parent tree is missing Message-ID: <5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn> ``` write_ondisk_index() dereferences the return value of repo_parse_tree_indirect() unconditionally. If the parent commit's tree object is missing from the object store (corrupt repository, object removed by tooling, or incomplete restore), the function returns NULL and "git history split" crashes with a SIGSEGV (release build; UBSan reports a null-pointer member access at builtin/history.c:789). Guard the parse result and error out gracefully, following the codebase convention for objects that cannot be loaded. Signed-off-by: Jinbao Chen --- Hi, (This was reported via the Git security contact, which suggested posting here. The security team asked me to post the fix on this list, as the crash requires a missing object in a local repository and is not considered a security issue.) "git history split" crashes with a SIGSEGV when the commit's parent tree object is missing from the object store (corrupt repository, object removed by tooling, incomplete backup/mirror restore): write_ondisk_index() dereferences the NULL return value of repo_parse_tree_indirect(). The fix below guards the parse result, matching the codebase convention for objects that cannot be loaded ("if (!tree) return error(...)"). Reproduction (verified on master @ f78ce2f7b6, x86-64 Linux): git init r && cd r git config user.email t@t && git config user.name t echo a > f && git add f && git commit -qm one echo b > f && git commit -qam two tree=$(git rev-parse 'HEAD^^{tree}') rm .git/objects/$(echo "$tree" | cut -c1-2)/$(echo "$tree" | cut -c3-) GIT_EDITOR=true git history split HEAD Before: release build SIGSEGV (exit 139, core dumped); UBSan reports "member access within null pointer of type 'struct tree'" at builtin/history.c:789. After: "error: unable to parse tree ", exit 255, no crash. Control (tree object present) is unchanged. 1 file changed, 4 insertions(+) diff --git a/builtin/history.c b/builtin/history.c index 000155ad9c..097631f5ba 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo, opts.dst_index = &index; tree = repo_parse_tree_indirect(repo, oid); + if (!tree) { + ret = error(_("unable to parse tree %s"), oid_to_hex(oid)); + goto out; + } init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size); if (unpack_trees(1, &tree_desc, &opts)) { -- 2.53.0 ``` ## Patrick Steinhardt, 2026-09-03 05:26 Subject: Re: [PATCH] history: do not dereference NULL when parent tree is missing Message-ID: In-Reply-To: <5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn> ``` On Wed, Sep 02, 2026 at 08:07:36PM +0800, zkd18cjb@mail.ustc.edu.cn wrote: > write_ondisk_index() dereferences the return value of > repo_parse_tree_indirect() unconditionally. If the parent commit's > tree object is missing from the object store (corrupt repository, > object removed by tooling, or incomplete restore), the function > returns NULL and "git history split" crashes with a SIGSEGV > (release build; UBSan reports a null-pointer member access at > builtin/history.c:789). Nit: the information in the braces does not really add a lot of signal, I'd just drop it. > Guard the parse result and error out gracefully, following the > codebase convention for objects that cannot be loaded. > > Signed-off-by: Jinbao Chen Nit: your From address does not match the Signed-off-by. [snip] > Reproduction (verified on master @ f78ce2f7b6, x86-64 Linux): > > git init r && cd r > git config user.email t@t && git config user.name t > echo a > f && git add f && git commit -qm one > echo b > f && git commit -qam two > tree=$(git rev-parse 'HEAD^^{tree}') > rm .git/objects/$(echo "$tree" | cut -c1-2)/$(echo "$tree" | cut -c3-) > GIT_EDITOR=true git history split HEAD We could of course add a test for this, but I don't really think that it's worth it. > diff --git a/builtin/history.c b/builtin/history.c > index 000155ad9c..097631f5ba 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo, > opts.dst_index = &index; > > tree = repo_parse_tree_indirect(repo, oid); > + if (!tree) { > + ret = error(_("unable to parse tree %s"), oid_to_hex(oid)); > + goto out; > + } Yup, the fix looks obviously good to me, thanks! Patrick ``` ## Jinbao Chen, 2026-09-03 06:36 Subject: Re: [PATCH v2] history: do not dereference NULL when parent tree is missing Message-ID: <20260903063657.2067303-1-zkd18cjb@mail.ustc.edu.cn> In-Reply-To: <5438d465.ab31e.1a062047bd5.Coremail.zkd18cjb@mail.ustc.edu.cn> ``` write_ondisk_index() dereferences the return value of repo_parse_tree_indirect() unconditionally. If the parent commit's tree object is missing from the object store (corrupt repository, object removed by tooling, or incomplete restore), the function returns NULL and "git history split" crashes with a SIGSEGV. Guard the parse result and error out gracefully, following the codebase convention for objects that cannot be loaded. Signed-off-by: Jinbao Chen --- Thanks for the review! Changes since v1 (no functional changes): - Dropped the parenthetical note about the UBSan diagnostic from the commit message, as suggested. - Sent with the From address matching the Signed-off-by. builtin/history.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/history.c b/builtin/history.c index 000155ad9c..097631f5ba 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo, opts.dst_index = &index; tree = repo_parse_tree_indirect(repo, oid); + if (!tree) { + ret = error(_("unable to parse tree %s"), oid_to_hex(oid)); + goto out; + } init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size); if (unpack_trees(1, &tree_desc, &opts)) { -- 2.53.0 ``` ## Patrick Steinhardt, 2026-09-03 07:52 Subject: Re: [PATCH v2] history: do not dereference NULL when parent tree is missing Message-ID: In-Reply-To: <20260903063657.2067303-1-zkd18cjb@mail.ustc.edu.cn> ``` On Thu, Sep 03, 2026 at 02:36:57PM +0800, Jinbao Chen wrote: > write_ondisk_index() dereferences the return value of > repo_parse_tree_indirect() unconditionally. If the parent commit's > tree object is missing from the object store (corrupt repository, > object removed by tooling, or incomplete restore), the function > returns NULL and "git history split" crashes with a SIGSEGV. > > Guard the parse result and error out gracefully, following the > codebase convention for objects that cannot be loaded. > > Signed-off-by: Jinbao Chen > --- > Thanks for the review! > > Changes since v1 (no functional changes): > - Dropped the parenthetical note about the UBSan diagnostic from the > commit message, as suggested. > - Sent with the From address matching the Signed-off-by. Thanks, this version looks good to me! Patrick ```