[PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

From

Michael Haggerty <mhagger@alum.mit.edu>

Date

Apr 27, 2016, 16:57 UTC

Message-ID

<a8e1e1a9e6a48fbb20fab2144279b93a48db584a.1461768689.git.mhagger@alum.mit.edu>

In-Reply-To

<cover.1461768689.git.mhagger@alum.mit.edu>

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Does anybody have a use case for allowing un-normalized reference
names like "refs/foo/../bar///baz"? I'm pretty certain they would not
be handled correctly anyway, especially if they are not stored as
loose references.

 refs.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index ca0280f..b18d995 100644
--- a/refs.c
+++ b/refs.c
@@ -125,14 +125,19 @@ int refname_is_safe(const char *refname)
 	if (skip_prefix(refname, "refs/", &rest)) {
 		char *buf;
 		int result;
+		size_t restlen = strlen(rest);
+
+		/* rest must not be empty, or start or end with "/" */
+		if (!restlen || *rest == '/' || rest[restlen - 1] == '/')
+			return 0;
 
 		/*
 		 * Does the refname try to escape refs/?
 		 * For example: refs/foo/../bar is safe but refs/foo/../../bar
 		 * is not.
 		 */
-		buf = xmallocz(strlen(rest));
-		result = !normalize_path_copy(buf, rest);
+		buf = xmallocz(restlen);
+		result = !normalize_path_copy(buf, rest) && !strcmp(buf, rest);
 		free(buf);
 		return result;
 	}

-- 
2.8.1