[PATCH] history: do not dereference NULL when parent tree is missing

[PATCH] history: do not dereference NULL when parent tree is missing

[zkd18cjb]

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 <zkd18cjb@mail.ustc.edu.cn>
---
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 <oid>", 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

Re: [PATCH] history: do not dereference NULL when parent tree is missing

[Patrick Steinhardt]

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 <zkd18cjb@mail.ustc.edu.cn>

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

Re: [PATCH v2] history: do not dereference NULL when parent tree is missing

[Jinbao Chen]

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 <zkd18cjb@mail.ustc.edu.cn>
---
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

Re: [PATCH v2] history: do not dereference NULL when parent tree is missing

[Patrick Steinhardt]

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 <zkd18cjb@mail.ustc.edu.cn>
> ---
> 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