threads / patch / 11454

patchreceive-pack: reject invalid refnames

Subject: [PATCH] receive-pack: reject invalid refnames

## tl;dr

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

replies: 1people: 2as markdown or json

Martin Koegler· Jan 2, 2008, 21:52 UTC · lore
Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 receive-pack.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Show changes to receive-pack.c +5 −2
diff --git a/receive-pack.c b/receive-pack.c
index d0a563d..a038a40 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -165,7 +165,9 @@ static const char *update(struct command *cmd)
 	unsigned char *new_sha1 = cmd->new_sha1;
 	struct ref_lock *lock;
 
-	if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
+	/* only HEAD and refs/... are allowed */
+	if (strcmp(name, "HEAD") && 
+	    (prefixcmp(name, "refs/") || check_ref_format(name + 5))) {
 		error("refusing to create funny ref '%s' remotely", name);
 		return "funny refname";
 	}
@@ -177,7 +179,8 @@ 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/")) {
+	    (!prefixcmp(name, "refs/heads/") ||
+	     !strcmp(name, "HEAD"))) {
 		struct object *old_object, *new_object;
 		struct commit *old_commit, *new_commit;
 		struct commit_list *bases, *ent;
-- 
1.4.4.4
Junio C Hamano· Jan 3, 2008, 21:40 UTC · re: Martin Koegler · lore

Re: [PATCH] receive-pack: reject invalid refnames

Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
Show 30 quoted lines
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
>  receive-pack.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/receive-pack.c b/receive-pack.c
> index d0a563d..a038a40 100644
> --- a/receive-pack.c
> +++ b/receive-pack.c
> @@ -165,7 +165,9 @@ static const char *update(struct command *cmd)
>  	unsigned char *new_sha1 = cmd->new_sha1;
>  	struct ref_lock *lock;
>  
> -	if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
> +	/* only HEAD and refs/... are allowed */
> +	if (strcmp(name, "HEAD") && 
> +	    (prefixcmp(name, "refs/") || check_ref_format(name + 5))) {
>  		error("refusing to create funny ref '%s' remotely", name);
>  		return "funny refname";
>  	}
> @@ -177,7 +179,8 @@ 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/")) {
> +	    (!prefixcmp(name, "refs/heads/") ||
> +	     !strcmp(name, "HEAD"))) {
>  		struct object *old_object, *new_object;
>  		struct commit *old_commit, *new_commit;
>  		struct commit_list *bases, *ent;
Yuck.  What I was smoking.

Normal client "git push" does not even allow "git push victim foo:HEAD". So if name is "HEAD" that has to be a malicious crafted push.

I think 
	if (prefixcmp(name, "refs/") || check_ref_format(name + 5))
		error();
is enough and correct.
Sorry for my earlier thinko.

← back to recent threads