Harden the request path, scan and error recovery

1 file changed, 79 insertions, 26 deletions

diff --git a/source/shared.c b/source/shared.cindex 6ac2353..a1d1597 100644--- a/source/shared.c+++ b/source/shared.c@@ -14,8 +14,8 @@  #define MACRO_EXPANSION_BUFSIZE (1024 * 8) -// The number of context lines git itself defaults to.-#define DEFAULT_DIFF_CONTEXT 3+// How much of a file is read to find its first line.+#define MAX_FIRST_LINE_READ (1024 * 64)  typedef struct { 	const char *name;@@ -68,9 +68,29 @@ static struct refinfo *make_refinfo(const char *refname, const struct object_id 	return ref; } +/*+ * Whether a revision from the request may reach git. A hex object id or a name+ * that could be a ref passes. Git's wider syntax is refused, since :/pattern+ * and rev^{/pattern} walk the whole history for a match, and a leading dash+ * reads as an option further down.+ */+int cgit_valid_rev(const char *rev)+{+	const char *p = rev;++	if (!rev || !*rev || *rev == '-')+		return 0;+	while (isxdigit((unsigned char)*p))+		p++;+	if (!*p)+		return 1;+	return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL);+}+ static int load_mmfile(mmfile_t *file, const struct object_id *oid) { 	enum object_type type;+	size_t size;  	// A null oid is the absent side of an add or a delete, and diffs as an 	// empty file rather than as a failure.@@ -80,10 +100,11 @@ static int load_mmfile(mmfile_t *file, const struct object_id *oid) 		return 1; 	} -	file->ptr = odb_read_object(the_repository->objects, oid, &type, (unsigned long *)&file->size);-	// odb_read_object leaves size untouched when it fails, so the caller-	// has to be told rather than handed a buffer with an unset length.-	return file->ptr != NULL;+	file->ptr = odb_read_object(the_repository->objects, oid, &type, &size);+	if (!file->ptr)+		return 0;+	file->size = size;+	return 1; }  /*@@ -106,7 +127,7 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf) 	int i;  	for (i = 0; i < nbuf; i++) {-		if (mb[i].ptr[mb[i].size-1] != '\n') {+		if (mb[i].ptr[mb[i].size - 1] != '\n') { 			fragment = xrealloc(fragment, fragment_len + mb[i].size); 			memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size); 			fragment_len += mb[i].size;@@ -133,8 +154,10 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf) 	return 0; } -// Takes an unsigned char because a byte over 0x7f is negative where char-// is signed, which the ctype tests are not defined for.+/*+ * Takes an unsigned char because a byte over 0x7f is negative where char is+ * signed, which the ctype tests are not defined for.+ */ static int is_token_char(unsigned char c) { 	return isalnum(c) || c == '_';@@ -192,14 +215,22 @@ struct cgit_repo *cgit_add_repo(const char *url) 			cgit_repolist.length = 8; 		else 			cgit_repolist.length *= 2;-		cgit_repolist.repos = xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo));+		cgit_repolist.repos =+			xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo)); 	} -	repo = &cgit_repolist.repos[cgit_repolist.count-1];+	repo = &cgit_repolist.repos[cgit_repolist.count - 1]; 	memset(repo, 0, sizeof(struct cgit_repo)); 	repo->url = cgit_trim_end(url, '/');-	if (repo->url)+	if (repo->url) { 		*strchrnul(repo->url, '\n') = '\0';+	} else {+		// Nothing can address a repository with no url, and every lookup+		// compares the url, so it is kept out of the way.+		fprintf(stderr, "[cgit] Ignoring repository with an empty url\n");+		repo->url = xstrdup("");+		repo->ignore = 1;+	} 	repo->name = repo->url; 	repo->path = NULL; 	repo->desc = cgit_default_repo_desc;@@ -222,7 +253,6 @@ struct cgit_repo *cgit_add_repo(const char *url) 	repo->branch_sort = ctx.cfg.branch_sort; 	repo->commit_sort = ctx.cfg.commit_sort; 	repo->module_link = ctx.cfg.module_link;-	repo->readme = ctx.cfg.readme; 	repo->mtime = -1; 	repo->about_filter = ctx.cfg.about_filter; 	repo->commit_filter = ctx.cfg.commit_filter;@@ -266,7 +296,7 @@ char *cgit_trim_end(const char *str, char c) { 	size_t len; -	if (str == NULL)+	if (!str) 		return NULL; 	len = strlen(str); 	while (len > 0 && str[len - 1] == c)@@ -286,7 +316,7 @@ char *cgit_ensure_end(const char *str, char c)  	result = xmalloc(len + 2); 	memcpy(result, str, len);-	result[len] = '/';+	result[len] = c; 	result[len + 1] = '\0'; 	return result; }@@ -367,9 +397,15 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new 	// Read the object headers first so an oversized blob is never inflated 	// into memory just to be diffed. Reporting it as binary suppresses 	// inlining the same way max-blob-size does in the other views.-	if (!is_null_oid(old_oid) && odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0)+	if (+		!is_null_oid(old_oid) &&+		odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0+	) 		return 1;-	if (!is_null_oid(new_oid) && odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0)+	if (+		!is_null_oid(new_oid) &&+		odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0+	) 		return 1;  	*old_size = old_bytes;@@ -389,7 +425,10 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new 		return 1; 	} -	if (buffer_is_binary(old_file.ptr, old_file.size) || buffer_is_binary(new_file.ptr, new_file.size)) {+	if (+		buffer_is_binary(old_file.ptr, old_file.size) ||+		buffer_is_binary(new_file.ptr, new_file.size)+	) { 		*binary = 1; 		release_mmfile(&old_file, old_oid); 		release_mmfile(&new_file, new_oid);@@ -431,6 +470,9 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new 		item = xcalloc(1, sizeof(*item)); 		item->match = xstrdup(prefix); 		item->len = strlen(prefix);+		// nowildcard_len matching len makes git treat the path as+		// literal rather than as a glob.+		item->nowildcard_len = item->len; 		opt.pathspec.nr = 1; 		opt.pathspec.items = item; 	}@@ -500,7 +542,7 @@ void cgit_prepare_repo_env(struct cgit_repo *repo)  	for (i = 0; i < ARRAY_SIZE(vars); i++) 		if (vars[i].value && setenv(vars[i].name, vars[i].value, 1))-			fprintf(stderr, "[cgit] Error setting env %s=%s: %s (%d)\n",+			fprintf(stderr, "[cgit] Unable to set %s=%s: %s (%d)\n", 				vars[i].name, vars[i].value, strerror(errno), errno); } @@ -508,6 +550,7 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) { 	int fd, err; 	ssize_t got;+	size_t want; 	struct stat st;  	fd = open(path, O_RDONLY);@@ -522,10 +565,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) 		close(fd); 		return EISDIR; 	}-	*buf = xmalloc(st.st_size + 1);-	got = read_in_full(fd, *buf, st.st_size);-	err = errno;+	// Only the first line is wanted, and a description file in a scanned+	// repository is whatever a pusher wrote, so the read is bounded.+	want = st.st_size < MAX_FIRST_LINE_READ ? st.st_size : MAX_FIRST_LINE_READ;+	*buf = xmalloc(want + 1);+	got = read_in_full(fd, *buf, want); 	if (got < 0) {+		err = errno; 		free(*buf); 		*buf = NULL; 		*size = 0;@@ -536,12 +582,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) 	(*buf)[*size] = '\0'; 	*strchrnul(*buf, '\n') = '\0'; 	close(fd);-	return (*size == (size_t)st.st_size ? 0 : err);+	return 0; }  char *cgit_strdup_first_line(const char *text) { 	char *line = xstrdup(text);+ 	*strchrnul(line, '\n') = '\0'; 	return line; }@@ -561,11 +608,13 @@ char *cgit_expand_macros(const char *text) 				if (out - start > 0) { 					*out = '\0'; 					out = expand_macro(start, limit - start) - 1;+					// Step back so the byte that ended the+					// token is written again and may open a+					// new one. A bare dollar expanded nothing+					// and its follower is already in place.+					text--; 				} 				start = NULL;-				// Step back so the byte that ended the token-				// is written again and may open a new one.-				text--; 			} 			out++; 			text++;@@ -621,6 +670,10 @@ char *cgit_get_mimetype_for_filename(const char *filename) 		// extension that maps to it. 		string_list_split_in_place(&fields, line, " \t\r\n", -1); 		string_list_remove_empty_items(&fields, 0);+		if (!fields.nr) {+			string_list_clear(&fields, 0);+			continue;+		} 		type = fields.items[0].string; 		for (i = 1; i < fields.nr; i++) { 			if (!strcasecmp(ext, fields.items[i].string)) {
diff --git a/source/shared.c b/source/shared.c
index 6ac2353..a1d1597 100644
--- a/source/shared.c
+++ b/source/shared.c
@@ -14,8 +14,8 @@
 
 #define MACRO_EXPANSION_BUFSIZE (1024 * 8)
 
-// The number of context lines git itself defaults to.
-#define DEFAULT_DIFF_CONTEXT 3
+// How much of a file is read to find its first line.
+#define MAX_FIRST_LINE_READ (1024 * 64)
 
 typedef struct {
 	const char *name;
@@ -68,9 +68,29 @@ static struct refinfo *make_refinfo(const char *refname, const struct object_id
 	return ref;
 }
 
+/*
+ * Whether a revision from the request may reach git. A hex object id or a name
+ * that could be a ref passes. Git's wider syntax is refused, since :/pattern
+ * and rev^{/pattern} walk the whole history for a match, and a leading dash
+ * reads as an option further down.
+ */
+int cgit_valid_rev(const char *rev)
+{
+	const char *p = rev;
+
+	if (!rev || !*rev || *rev == '-')
+		return 0;
+	while (isxdigit((unsigned char)*p))
+		p++;
+	if (!*p)
+		return 1;
+	return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL);
+}
+
 static int load_mmfile(mmfile_t *file, const struct object_id *oid)
 {
 	enum object_type type;
+	size_t size;
 
 	// A null oid is the absent side of an add or a delete, and diffs as an
 	// empty file rather than as a failure.
@@ -80,10 +100,11 @@ static int load_mmfile(mmfile_t *file, const struct object_id *oid)
 		return 1;
 	}
 
-	file->ptr = odb_read_object(the_repository->objects, oid, &type, (unsigned long *)&file->size);
-	// odb_read_object leaves size untouched when it fails, so the caller
-	// has to be told rather than handed a buffer with an unset length.
-	return file->ptr != NULL;
+	file->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
+	if (!file->ptr)
+		return 0;
+	file->size = size;
+	return 1;
 }
 
 /*
@@ -106,7 +127,7 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 	int i;
 
 	for (i = 0; i < nbuf; i++) {
-		if (mb[i].ptr[mb[i].size-1] != '\n') {
+		if (mb[i].ptr[mb[i].size - 1] != '\n') {
 			fragment = xrealloc(fragment, fragment_len + mb[i].size);
 			memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size);
 			fragment_len += mb[i].size;
@@ -133,8 +154,10 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 	return 0;
 }
 
-// Takes an unsigned char because a byte over 0x7f is negative where char
-// is signed, which the ctype tests are not defined for.
+/*
+ * Takes an unsigned char because a byte over 0x7f is negative where char is
+ * signed, which the ctype tests are not defined for.
+ */
 static int is_token_char(unsigned char c)
 {
 	return isalnum(c) || c == '_';
@@ -192,14 +215,22 @@ struct cgit_repo *cgit_add_repo(const char *url)
 			cgit_repolist.length = 8;
 		else
 			cgit_repolist.length *= 2;
-		cgit_repolist.repos = xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo));
+		cgit_repolist.repos =
+			xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo));
 	}
 
-	repo = &cgit_repolist.repos[cgit_repolist.count-1];
+	repo = &cgit_repolist.repos[cgit_repolist.count - 1];
 	memset(repo, 0, sizeof(struct cgit_repo));
 	repo->url = cgit_trim_end(url, '/');
-	if (repo->url)
+	if (repo->url) {
 		*strchrnul(repo->url, '\n') = '\0';
+	} else {
+		// Nothing can address a repository with no url, and every lookup
+		// compares the url, so it is kept out of the way.
+		fprintf(stderr, "[cgit] Ignoring repository with an empty url\n");
+		repo->url = xstrdup("");
+		repo->ignore = 1;
+	}
 	repo->name = repo->url;
 	repo->path = NULL;
 	repo->desc = cgit_default_repo_desc;
@@ -222,7 +253,6 @@ struct cgit_repo *cgit_add_repo(const char *url)
 	repo->branch_sort = ctx.cfg.branch_sort;
 	repo->commit_sort = ctx.cfg.commit_sort;
 	repo->module_link = ctx.cfg.module_link;
-	repo->readme = ctx.cfg.readme;
 	repo->mtime = -1;
 	repo->about_filter = ctx.cfg.about_filter;
 	repo->commit_filter = ctx.cfg.commit_filter;
@@ -266,7 +296,7 @@ char *cgit_trim_end(const char *str, char c)
 {
 	size_t len;
 
-	if (str == NULL)
+	if (!str)
 		return NULL;
 	len = strlen(str);
 	while (len > 0 && str[len - 1] == c)
@@ -286,7 +316,7 @@ char *cgit_ensure_end(const char *str, char c)
 
 	result = xmalloc(len + 2);
 	memcpy(result, str, len);
-	result[len] = '/';
+	result[len] = c;
 	result[len + 1] = '\0';
 	return result;
 }
@@ -367,9 +397,15 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new
 	// Read the object headers first so an oversized blob is never inflated
 	// into memory just to be diffed. Reporting it as binary suppresses
 	// inlining the same way max-blob-size does in the other views.
-	if (!is_null_oid(old_oid) && odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0)
+	if (
+		!is_null_oid(old_oid) &&
+		odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0
+	)
 		return 1;
-	if (!is_null_oid(new_oid) && odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0)
+	if (
+		!is_null_oid(new_oid) &&
+		odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0
+	)
 		return 1;
 
 	*old_size = old_bytes;
@@ -389,7 +425,10 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new
 		return 1;
 	}
 
-	if (buffer_is_binary(old_file.ptr, old_file.size) || buffer_is_binary(new_file.ptr, new_file.size)) {
+	if (
+		buffer_is_binary(old_file.ptr, old_file.size) ||
+		buffer_is_binary(new_file.ptr, new_file.size)
+	) {
 		*binary = 1;
 		release_mmfile(&old_file, old_oid);
 		release_mmfile(&new_file, new_oid);
@@ -431,6 +470,9 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new
 		item = xcalloc(1, sizeof(*item));
 		item->match = xstrdup(prefix);
 		item->len = strlen(prefix);
+		// nowildcard_len matching len makes git treat the path as
+		// literal rather than as a glob.
+		item->nowildcard_len = item->len;
 		opt.pathspec.nr = 1;
 		opt.pathspec.items = item;
 	}
@@ -500,7 +542,7 @@ void cgit_prepare_repo_env(struct cgit_repo *repo)
 
 	for (i = 0; i < ARRAY_SIZE(vars); i++)
 		if (vars[i].value && setenv(vars[i].name, vars[i].value, 1))
-			fprintf(stderr, "[cgit] Error setting env %s=%s: %s (%d)\n",
+			fprintf(stderr, "[cgit] Unable to set %s=%s: %s (%d)\n",
 				vars[i].name, vars[i].value, strerror(errno), errno);
 }
 
@@ -508,6 +550,7 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 {
 	int fd, err;
 	ssize_t got;
+	size_t want;
 	struct stat st;
 
 	fd = open(path, O_RDONLY);
@@ -522,10 +565,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 		close(fd);
 		return EISDIR;
 	}
-	*buf = xmalloc(st.st_size + 1);
-	got = read_in_full(fd, *buf, st.st_size);
-	err = errno;
+	// Only the first line is wanted, and a description file in a scanned
+	// repository is whatever a pusher wrote, so the read is bounded.
+	want = st.st_size < MAX_FIRST_LINE_READ ? st.st_size : MAX_FIRST_LINE_READ;
+	*buf = xmalloc(want + 1);
+	got = read_in_full(fd, *buf, want);
 	if (got < 0) {
+		err = errno;
 		free(*buf);
 		*buf = NULL;
 		*size = 0;
@@ -536,12 +582,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 	(*buf)[*size] = '\0';
 	*strchrnul(*buf, '\n') = '\0';
 	close(fd);
-	return (*size == (size_t)st.st_size ? 0 : err);
+	return 0;
 }
 
 char *cgit_strdup_first_line(const char *text)
 {
 	char *line = xstrdup(text);
+
 	*strchrnul(line, '\n') = '\0';
 	return line;
 }
@@ -561,11 +608,13 @@ char *cgit_expand_macros(const char *text)
 				if (out - start > 0) {
 					*out = '\0';
 					out = expand_macro(start, limit - start) - 1;
+					// Step back so the byte that ended the
+					// token is written again and may open a
+					// new one. A bare dollar expanded nothing
+					// and its follower is already in place.
+					text--;
 				}
 				start = NULL;
-				// Step back so the byte that ended the token
-				// is written again and may open a new one.
-				text--;
 			}
 			out++;
 			text++;
@@ -621,6 +670,10 @@ char *cgit_get_mimetype_for_filename(const char *filename)
 		// extension that maps to it.
 		string_list_split_in_place(&fields, line, " \t\r\n", -1);
 		string_list_remove_empty_items(&fields, 0);
+		if (!fields.nr) {
+			string_list_clear(&fields, 0);
+			continue;
+		}
 		type = fields.items[0].string;
 		for (i = 1; i < fields.nr; i++) {
 			if (!strcasecmp(ext, fields.items[i].string)) {