Re: [RFC Patch] Preventing corrupt objects from entering the repository

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 13, 2008 at 12:01:43PM +0000, Johannes Schindelin wrote:
> On Wed, 13 Feb 2008, Martin Koegler wrote:
> > This would mean, that we must make git-rev-list and git-pack-objects not 
> > segfault on incorrect links between objects.
> 
> We should do that anyway.  It may error out, but segfaulting is no option.
> 
> So if you have a test case, please make it public so we can fix the 
> breakage.

OK, here is the first part of missing checks. 

Its still WIP:
* It should be checked, if reactions to errors are correct.
* Patch should be splitted and turned into submitable pieces

If anybody wants to do this, please go on.

>From 4a6908ff51059a5b2bcc8d8c41fd41de53ee9151 Mon Sep 17 00:00:00 2001
From: Martin Koegler <mkoegler@xxxxxxxxxxxxxxxxx>
Date: Thu, 14 Feb 2008 19:34:39 +0100
Subject: [PATCH] Missing checks (1)

Signed-off-by: Martin Koegler <mkoegler@xxxxxxxxxxxxxxxxx>
---
 commit.c    |   16 +++++++++++-----
 revision.c  |   12 ++++++++++++
 sha1_file.c |    3 ++-
 sha1_name.c |   11 ++++++++++-
 tag.c       |    5 ++++-
 5 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/commit.c b/commit.c
index 8b8fb04..53f0042 100644
--- a/commit.c
+++ b/commit.c
@@ -311,6 +311,8 @@ int parse_commit(struct commit *item)
 	unsigned long size;
 	int ret;
 
+	if (!item)
+		return -1;
 	if (item->object.parsed)
 		return 0;
 	buffer = read_sha1_file(item->object.sha1, &type, &size);
@@ -385,8 +387,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
 
 	while (parents) {
 		struct commit *commit = parents->item;
-		parse_commit(commit);
-		if (!(commit->object.flags & mark)) {
+		if (!parse_commit(commit) && !(commit->object.flags & mark)) {
 			commit->object.flags |= mark;
 			insert_by_date(commit, list);
 		}
@@ -552,8 +553,10 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
 		 */
 		return commit_list_insert(one, &result);
 
-	parse_commit(one);
-	parse_commit(two);
+	if (parse_commit(one))
+		die("invalid commit");
+	if (parse_commit(two))
+		die("invalid commit");
 
 	one->object.flags |= PARENT1;
 	two->object.flags |= PARENT2;
@@ -584,9 +587,12 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
 		while (parents) {
 			struct commit *p = parents->item;
 			parents = parents->next;
+			if (!p)
+				die("invalid commit");
 			if ((p->object.flags & flags) == flags)
 				continue;
-			parse_commit(p);
+			if(parse_commit(p))
+				die("invalid commit");
 			p->object.flags |= flags;
 			insert_by_date(p, &list);
 		}
diff --git a/revision.c b/revision.c
index 6e85aaa..9f8723d 100644
--- a/revision.c
+++ b/revision.c
@@ -46,6 +46,8 @@ void add_object(struct object *obj,
 
 static void mark_blob_uninteresting(struct blob *blob)
 {
+	if (!blob)
+		return;
 	if (blob->object.flags & UNINTERESTING)
 		return;
 	blob->object.flags |= UNINTERESTING;
@@ -57,6 +59,8 @@ void mark_tree_uninteresting(struct tree *tree)
 	struct name_entry entry;
 	struct object *obj = &tree->object;
 
+	if (!obj)
+		return;
 	if (obj->flags & UNINTERESTING)
 		return;
 	obj->flags |= UNINTERESTING;
@@ -94,6 +98,8 @@ void mark_parents_uninteresting(struct commit *commit)
 
 	while (parents) {
 		struct commit *commit = parents->item;
+		if (!commit)
+			continue;
 		if (!(commit->object.flags & UNINTERESTING)) {
 			commit->object.flags |= UNINTERESTING;
 
@@ -173,6 +179,8 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
 		struct tag *tag = (struct tag *) object;
 		if (revs->tag_objects && !(flags & UNINTERESTING))
 			add_pending_object(revs, object, tag->tag);
+		if (!tag->tagged)
+			die("bad tag");
 		object = parse_object(tag->tagged->sha1);
 		if (!object)
 			die("bad object %s", sha1_to_hex(tag->tagged->sha1));
@@ -685,12 +693,16 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 		it = get_reference(revs, arg, sha1, 0);
 		if (it->type != OBJ_TAG)
 			break;
+		if (!((struct tag*)it)->tagged)
+			return 0;
 		hashcpy(sha1, ((struct tag*)it)->tagged->sha1);
 	}
 	if (it->type != OBJ_COMMIT)
 		return 0;
 	commit = (struct commit *)it;
 	for (parents = commit->parents; parents; parents = parents->next) {
+		if (!parents->item)
+			continue;
 		it = &parents->item->object;
 		it->flags |= flags;
 		add_pending_object(revs, it, arg);
diff --git a/sha1_file.c b/sha1_file.c
index 4179949..c25ce64 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1943,7 +1943,8 @@ void *read_object_with_reference(const unsigned char *sha1,
 		}
 		ref_length = strlen(ref_type);
 
-		if (memcmp(buffer, ref_type, ref_length) ||
+		if (ref_length + 40 >= size ||
+		    memcmp(buffer, ref_type, ref_length) ||
 		    get_sha1_hex((char *) buffer + ref_length, actual_sha1)) {
 			free(buffer);
 			return NULL;
diff --git a/sha1_name.c b/sha1_name.c
index be8489e..bd0bce6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -396,6 +396,8 @@ static int get_parent(const char *name, int len,
 	p = commit->parents;
 	while (p) {
 		if (!--idx) {
+			if (!p->item)
+				return -1;
 			hashcpy(result, p->item->object.sha1);
 			return 0;
 		}
@@ -417,6 +419,8 @@ static int get_nth_ancestor(const char *name, int len,
 
 		if (!commit || parse_commit(commit) || !commit->parents)
 			return -1;
+		if (!commit->parents->item)
+			return -1;
 		hashcpy(sha1, commit->parents->item->object.sha1);
 	}
 	hashcpy(result, sha1);
@@ -494,6 +498,8 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 				return error("%.*s: expected %s type, but the object dereferences to %s type",
 					     len, name, typename(expected_type),
 					     typename(o->type));
+			if (!o)
+				return -1;
 			if (!o->parsed)
 				parse_object(o->sha1);
 		}
@@ -580,6 +586,8 @@ static int handle_one_ref(const char *path,
 		return 0;
 	if (object->type == OBJ_TAG)
 		object = deref_tag(object, path, strlen(path));
+	if (!object)
+		return 0;
 	if (object->type != OBJ_COMMIT)
 		return 0;
 	insert_by_date((struct commit *)object, list);
@@ -617,7 +625,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 		unsigned long size;
 
 		commit = pop_most_recent_commit(&list, ONELINE_SEEN);
-		parse_object(commit->object.sha1);
+		if(!parse_object(commit->object.sha1))
+			continue;
 		if (temp_commit_buffer)
 			free(temp_commit_buffer);
 		if (commit->buffer)
diff --git a/tag.c b/tag.c
index 38bf913..990134f 100644
--- a/tag.c
+++ b/tag.c
@@ -9,7 +9,10 @@ const char *tag_type = "tag";
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
 	while (o && o->type == OBJ_TAG)
-		o = parse_object(((struct tag *)o)->tagged->sha1);
+		if (((struct tag *)o)->tagged)
+			o = parse_object(((struct tag *)o)->tagged->sha1);
+		else
+			o = NULL;
 	if (!o && warn) {
 		if (!warnlen)
 			warnlen = strlen(warn);
-- 
1.5.4.1.gb43a

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux