Serve only the files a dumb clone asks for

The object path check let any run of safe characters through, so a
spelling such as info//alternates or info/./alternates reached the
alternates file the exact name was refused for.

1 file changed, 21 insertions, 16 deletions

diff --git a/source/ui-clone.c b/source/ui-clone.cindex 610aaa6..dad21d5 100644--- a/source/ui-clone.c+++ b/source/ui-clone.c@@ -79,20 +79,31 @@ static void print_pack_info(void) 	} } +static size_t hex_span(const char *s)+{+	return strspn(s, "0123456789abcdef");+}+ /*- * Only the characters an object path can hold pass, and no dotdot component.+ * A dumb client asks for three shapes of file below objects: a loose object+ * at xx/rest-of-hex, a pack or its index at pack/pack-hex.pack or .idx, and+ * info/http-alternates. Nothing else is served, so no spelling of a path can+ * reach a file outside those, the alternates file among them.  */-static int path_is_safe(const char *path)+static int object_path_is_served(const char *path) {-	const char *p;+	const size_t hexsz = the_hash_algo->hexsz;+	const char *rest; -	for (p = path; *p; ++p) {-		if (*p == '.' && *(p + 1) == '.')-			return 0;-		if (!isalnum((unsigned char)*p) && *p != '/' && *p != '.' && *p != '-')-			return 0;+	if (!strcmp(path, "info/http-alternates"))+		return 1;+	if (skip_prefix(path, "pack/pack-", &rest)) {+		size_t n = hex_span(rest);++		return n == hexsz && (!strcmp(rest + n, ".pack") || !strcmp(rest + n, ".idx")); 	}-	return 1;+	return hex_span(path) == 2 && path[2] == '/' && hex_span(path + 3) == hexsz - 2 &&+		path[hexsz + 1] == '\0'; }  /*@@ -166,17 +177,11 @@ void cgit_clone_objects(void) 		return; 	} -	// The alternates file names directories on the server's disk, which a-	// client fetching over http cannot reach, so only the http form that-	// is written for such clients goes out.-	if (!strcmp(ctx.qry.path, "info/alternates")) {+	if (!object_path_is_served(ctx.qry.path)) { 		cgit_print_error_page(404, "Not Found", "Not found"); 		return; 	} -	if (!path_is_safe(ctx.qry.path))-		goto err;- 	path = repo_git_path(the_repository, "objects/%s", ctx.qry.path); 	send_file(path); 	free(path);
diff --git a/source/ui-clone.c b/source/ui-clone.c
index 610aaa6..dad21d5 100644
--- a/source/ui-clone.c
+++ b/source/ui-clone.c
@@ -79,20 +79,31 @@ static void print_pack_info(void)
 	}
 }
 
+static size_t hex_span(const char *s)
+{
+	return strspn(s, "0123456789abcdef");
+}
+
 /*
- * Only the characters an object path can hold pass, and no dotdot component.
+ * A dumb client asks for three shapes of file below objects: a loose object
+ * at xx/rest-of-hex, a pack or its index at pack/pack-hex.pack or .idx, and
+ * info/http-alternates. Nothing else is served, so no spelling of a path can
+ * reach a file outside those, the alternates file among them.
  */
-static int path_is_safe(const char *path)
+static int object_path_is_served(const char *path)
 {
-	const char *p;
+	const size_t hexsz = the_hash_algo->hexsz;
+	const char *rest;
 
-	for (p = path; *p; ++p) {
-		if (*p == '.' && *(p + 1) == '.')
-			return 0;
-		if (!isalnum((unsigned char)*p) && *p != '/' && *p != '.' && *p != '-')
-			return 0;
+	if (!strcmp(path, "info/http-alternates"))
+		return 1;
+	if (skip_prefix(path, "pack/pack-", &rest)) {
+		size_t n = hex_span(rest);
+
+		return n == hexsz && (!strcmp(rest + n, ".pack") || !strcmp(rest + n, ".idx"));
 	}
-	return 1;
+	return hex_span(path) == 2 && path[2] == '/' && hex_span(path + 3) == hexsz - 2 &&
+		path[hexsz + 1] == '\0';
 }
 
 /*
@@ -166,17 +177,11 @@ void cgit_clone_objects(void)
 		return;
 	}
 
-	// The alternates file names directories on the server's disk, which a
-	// client fetching over http cannot reach, so only the http form that
-	// is written for such clients goes out.
-	if (!strcmp(ctx.qry.path, "info/alternates")) {
+	if (!object_path_is_served(ctx.qry.path)) {
 		cgit_print_error_page(404, "Not Found", "Not found");
 		return;
 	}
 
-	if (!path_is_safe(ctx.qry.path))
-		goto err;
-
 	path = repo_git_path(the_repository, "objects/%s", ctx.qry.path);
 	send_file(path);
 	free(path);