# [PATCH] receive-pack: check object type of sha1 before using them as commits 3 messages from 2008-01-02 to 2008-01-02. Participants: Martin Koegler, Linus Torvalds, Junio C Hamano. Thread: https://gitlist.dev/t/11447 ## Martin Koegler, 2008-01-02 07:39 Subject: [PATCH] receive-pack: check object type of sha1 before using them as commits Message-ID: <11992595612601-git-send-email-mkoegler@auto.tuwien.ac.at> URL: https://gitlist.dev/e/11992595612601-git-send-email-mkoegler%40auto.tuwien.ac.at ``` Signed-off-by: Martin Koegler --- receive-pack.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/receive-pack.c b/receive-pack.c index fba4cf8..d0a563d 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -178,11 +178,21 @@ static const char *update(struct command *cmd) if (deny_non_fast_forwards && !is_null_sha1(new_sha1) && !is_null_sha1(old_sha1) && !prefixcmp(name, "refs/heads/")) { + struct object *old_object, *new_object; struct commit *old_commit, *new_commit; struct commit_list *bases, *ent; - old_commit = (struct commit *)parse_object(old_sha1); - new_commit = (struct commit *)parse_object(new_sha1); + old_object = parse_object(old_sha1); + new_object = parse_object(new_sha1); + + if (!old_object || !new_object || + old_object->type != OBJ_COMMIT || + new_object->type != OBJ_COMMIT) { + error("bad sha1 objects for %s", name); + return "bad ref"; + } + old_commit = (struct commit *)old_object; + new_commit = (struct commit *)new_object; bases = get_merge_bases(old_commit, new_commit, 1); for (ent = bases; ent; ent = ent->next) if (!hashcmp(old_sha1, ent->item->object.sha1)) -- 1.4.4.4 ``` ## Linus Torvalds, 2008-01-02 22:21 Subject: Re: [PATCH] receive-pack: check object type of sha1 before using them as commits Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.00.0801021418510.3010%40woody.linux-foundation.org In-Reply-To: <11992595612601-git-send-email-mkoegler@auto.tuwien.ac.at> ``` On Wed, 2 Jan 2008, Martin Koegler wrote: > > - old_commit = (struct commit *)parse_object(old_sha1); > - new_commit = (struct commit *)parse_object(new_sha1); > + old_object = parse_object(old_sha1); > + new_object = parse_object(new_sha1); I think it would be better to use old_object = lookup_commit_reference(old_sha1); if (!old_object) return "bad ref"; new_object = lookup_commit_reference(new_sha1); if (!new_object) return "bad ref"; which will write a slightly more useful error message if it's not a commit (ie it will use the "check_commit()" function in commit.c) Linus ``` ## Junio C Hamano, 2008-01-02 22:57 Subject: Re: [PATCH] receive-pack: check object type of sha1 before using them as commits Message-ID: <7vprwjvns0.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vprwjvns0.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Linus Torvalds writes: > On Wed, 2 Jan 2008, Martin Koegler wrote: >> >> - old_commit = (struct commit *)parse_object(old_sha1); >> - new_commit = (struct commit *)parse_object(new_sha1); >> + old_object = parse_object(old_sha1); >> + new_object = parse_object(new_sha1); > > I think it would be better to use > > old_object = lookup_commit_reference(old_sha1); > if (!old_object) > return "bad ref"; > new_object = lookup_commit_reference(new_sha1); > if (!new_object) > return "bad ref"; > > which will write a slightly more useful error message if it's not a commit > (ie it will use the "check_commit()" function in commit.c) Hmmm... Three points. * lookup_commit_reference() is silent if the object pointed at by old/new SHA-1 is missing. * when storing in refs/heads, we do not want to have committish but we do want an actual commit object. * returning like that the user cannot tell which ref had the error. ```