threads / patch / 11447

patchreceive-pack: check object type of sha1 before using them as commits

Subject: [PATCH] receive-pack: check object type of sha1 before using them as commits

## tl;dr

3 messages between Jan 2, 2008 and Jan 2, 2008. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Martin Koegler· Jan 2, 2008, 07:39 UTC · lore
Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 receive-pack.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)
Show changes to receive-pack.c +12 −2
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· Jan 2, 2008, 22:21 UTC · re: Martin Koegler · lore

Re: [PATCH] receive-pack: check object type of sha1 before using them as commits

On Wed, 2 Jan 2008, Martin Koegler wrote:
Show 5 quoted lines
>  
> -		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· Jan 2, 2008, 22:57 UTC · re: Linus Torvalds · lore

Re: [PATCH] receive-pack: check object type of sha1 before using them as commits

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 18 quoted lines
> 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.

← back to recent threads