shithub: riscv

Info  •  Files  •  Log  •  Branches

git/pull: validate and prevent rc from checking reference names (thanks falsifian)

1. If the remote $origin has refs under refs/remotes/$origin,
these should be ignored, but instead, git/pull saves them
to .git/refs/remotes/$origin, possibly overwriting what it
found under refs/heads in the remote.

As a concrete example, the main branch for https://github.com/Perl/perl5.git
is called blead. If I run git/pull in a local checkout, I end up
with an outdated blead because for some reason the github repo has
its own refs/remotes/origin/blead, and git/get prints it after
refs/heads/blead.

2. If the remote path has special characters like '}', the remote
server could probably trick git/pull into executing arbitrary rc
commands. I haven't actually verified this is possible, but git/pull
is pasting the output of git/get unescaped into an rc command, and
I couldn't see any safety checking for that value in git/get.c.

This patch fixes both issues.

--- a/sys/src/cmd/git/get.c

+++ b/sys/src/cmd/git/get.c

@@ -7,7 +7,21 @@

 char *upstream = "origin";

 int listonly;

+/*

+ * Checks the rules for a refname at

+ * git/Documentation/protocol-common.txt

+ */

 int

+okrefname(char *s)

+{

+	if(strcmp(s, "HEAD") == 0)

+		return 1;

+	if(strncmp(s, "refs/", 5) == 0)

+		return okref(s);

+	return 0;

+}

+

+int

 resolveremote(Hash *h, char *ref)

 {

 	char buf[128], *s;

@@ -247,6 +261,8 @@

 		getfields(buf, sp, nelem(sp), 1, " \t\n\r");

 		if(strstr(sp[1], "^{}"))

 			continue;

+		if(!okrefname(sp[1]))

+			sysfatal("remote side sent invalid ref: %s", sp[1]);

 		if(fetchbranch && !branchmatch(sp[1], fetchbranch))

 			continue;

 		if(refsz == nref + 1){

--- a/sys/src/cmd/git/git.h

+++ b/sys/src/cmd/git/git.h

@@ -338,6 +338,7 @@

 int	writephase(Conn *);

 void	closeconn(Conn *);

 void	parsecaps(char *, Conn *);

+int	okref(char*);

 /* queues */

 void	qinit(Objq*);

--- a/sys/src/cmd/git/pull

+++ b/sys/src/cmd/git/pull

@@ -10,14 +10,14 @@

 	if(! ~ $#debug 0)

 		dflag='-d'

 	{git/get $dflag -u $upstream $url >[2=3] || die $status} | awk '

-	/^remote/{

-		if($2=="HEAD")

-			next

+	/^remote refs\/(heads|tags)\//{

 		ref=$2

 		hash=$3

 		gsub("^refs/heads", "refs/remotes/'$upstream'", ref)

 		outfile = ".git/"ref

-		system("mkdir -p `{basename -d "outfile"}");

+		print outfile > "/env/outfile"

+		system("mkdir -p `$nl{basename -d $outfile}");\

+		close("/env/outfile")

 		print hash > outfile;

 		close(outfile);

 	}

--- a/sys/src/cmd/git/util.c

+++ b/sys/src/cmd/git/util.c

@@ -513,3 +513,56 @@

 		sysfatal("corrupt qid: %s (%x)", s, *e);

 	return q;

 }

+

+/*

+ * Checks the rules for valid ref names, as defined in

+ *   git/Documentation/protocol-common.txt.

+ * It does not check that the ref begins with refs/

+ * or is called HEAD, since that's only needed in the

+ * clone protocol.

+ */

+int

+okref(char *ref)

+{

+	int slashed;

+	char *p;

+

+	slashed = 0;

+	if(*ref == '/')

+		return 0;

+	for(p = ref; *p != 0; p++) {

+		switch(*p){

+		case '.':

+			if(p[1]== 0 || p[1] == '.')

+				return 0;

+			if(strcmp(p, ".lock") == 0)

+				return 0;

+			break;

+		case '/':

+			if(p[1] == 0 || p[1] == '.')

+				return 0;

+			slashed = 1;

+			break;

+		case '@':

+			if(p[1] == '{')

+				return 0;

+			break;

+		case ' ':

+		case '~':

+		case '^':

+		case ':':

+		case '?':

+		case '*':

+		case '[':

+		case '\\':

+		case 0x7f: /* DEL */

+			return 0;

+		default:

+			if(*p < 0x20)

+				return 0;

+		}

+	}

+	if(ref == p)

+		return 0;

+	return slashed;

+}

-- 

⑨