Am 25.08.19 um 01:09 schrieb Stefan Sperling: > A tag object which lacks newlines won't be parsed correctly. > Git fails to detect this error and crashes due to a NULL deref: > > $ git archive 1.0.0 > Segmentation fault (core dumped) > $ git checkout 1.0.0 > Segmentation fault (core dumped) > $
Good find. > > See the attached tarball for a reproduction repository. > Also mirrored at https://stsp.name/git-checkout-tag-segv-repo.tgz > > With the patch below: > > $ git checkout 1.0.0 > fatal: reference is not a tree: 1.0.0 > $ git archive 1.0.0 > fatal: not a tree object: a99665eea5ee50171b5b7249880aa2ae35e35823 > $ Sign-off? > > diff --git a/tree.c b/tree.c > index 4720945e6a..92d8bd57a3 100644 > --- a/tree.c > +++ b/tree.c > @@ -252,9 +252,11 @@ struct tree *parse_tree_indirect(const struct object_id > *oid) > return (struct tree *) obj; > else if (obj->type == OBJ_COMMIT) > obj = &(get_commit_tree(((struct commit > *)obj))->object); > - else if (obj->type == OBJ_TAG) > + else if (obj->type == OBJ_TAG) { > obj = ((struct tag *) obj)->tagged; > - else > + if (!obj) > + return NULL; > + } else OK. There seem to be some more placed the use ->tagged without checking (found with "git grep -wW tagged"): builtin/describe.c::describe_commit() builtin/fast-export.c::handle_tag() builtin/log.c::cmd_show() builtin/replace.c::check_one_mergetag() fsck.c::fsck_walk_tag() -- I'm not sure about that one log-tree.c::show_one_mergetag() packfile.c::add_promisor_object() ref-filter.c::populate_value() ref-filter.c::match_points_at() walker.c::process_tag() Ugh! Do you perhaps want to have a go at them as well? > return NULL; > if (!obj->parsed) > parse_object(the_repository, &obj->oid); > Hmm, I find it a bit sad that this function is almost a duplicate of sha1-name.c::repo_peel_to_type(), which already checks for ->tagged being NULL. René
