Harden the page renderers

1 file changed, 71 insertions, 26 deletions

diff --git a/source/ui-log.c b/source/ui-log.c
index 316df75..86cb756 100644
--- a/source/ui-log.c
+++ b/source/ui-log.c
@@ -176,6 +176,9 @@ static void wrap_subject(struct commitinfo *info, struct strbuf *msg)
 		--cut;
 	if (!cut)
 		cut = ctx.cfg.max_msg_len - strlen(wrap_symbol);
+	// A cut inside a multibyte character would leave both halves invalid.
+	while (cut > 0 && (info->subject[cut] & 0xC0) == 0x80)
+		--cut;
 
 	strbuf_add(msg, info->subject + cut, subject_len - cut);
 	strbuf_trim(msg);
@@ -244,7 +247,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 		oid_to_hex(&commit->object.oid), ctx.qry.vpath);
 	cgit_print_commit_decorations(commit);
 	html("</td><td class='col-author'>");
-	cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
+	cgit_open_filter(ctx.repo->email_filter, info->author_email ? info->author_email : "", "log");
 	html_txt(info->author);
 	cgit_close_filter(ctx.repo->email_filter);
 
@@ -286,8 +289,9 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 			int msg_lines = ctx.qry.showmsg ? line_count(msgbuf.buf) : 0;
 
 			print_graph_padding(revs, &graphbuf, msg_lines);
-		} else
+		} else {
 			html("<td></td>");
+		}
 
 		// Either way one cell is already on the row, so the message
 		// spans the remaining columns.
@@ -317,6 +321,27 @@ static const char *disambiguate_ref(const char *ref, int *must_free_result)
 	return ref;
 }
 
+/*
+ * A range token is one revision, or two joined by two or three dots, and each
+ * side has to pass cgit_valid_rev. An empty side means HEAD to git.
+ */
+static int valid_range_token(const char *arg)
+{
+	const char *dots = strstr(arg, "..");
+	char *left;
+	int ok;
+
+	if (!dots)
+		return cgit_valid_rev(arg);
+	left = xstrndup(arg, dots - arg);
+	dots += 2;
+	if (*dots == '.')
+		dots++;
+	ok = (!*left || cgit_valid_rev(left)) && (!*dots || cgit_valid_rev(dots));
+	free(left);
+	return ok;
+}
+
 static char *next_token(char **src)
 {
 	char *token;
@@ -348,7 +373,7 @@ static void print_pager(struct rev_info *revs, int ofs, int cnt)
 	struct commit *more = get_revision(revs);
 
 	html("</table>\n");
-	// A single page needs no pager, and an empty list is just noise.
+	// A single page needs no pager, and an empty list has nothing to page.
 	if (ofs <= 0 && !more)
 		return;
 	html("<ul class='pager'>");
@@ -378,7 +403,7 @@ static void print_pager(struct rev_info *revs, int ofs, int cnt)
 void cgit_print_commit_decorations(struct commit *commit)
 {
 	const struct name_decoration *deco;
-	static char buf[1024];
+	const char *buf;
 
 	deco = get_name_decoration(&commit->object);
 	if (!deco)
@@ -388,12 +413,13 @@ void cgit_print_commit_decorations(struct commit *commit)
 		struct object_id oid_tag, peeled;
 		int is_annotated = 0;
 
-		strlcpy(buf, prettify_refname(deco->name), sizeof(buf));
+		buf = prettify_refname(deco->name);
 		switch (deco->type) {
 		case DECORATION_NONE:
 			break;
 		case DECORATION_REF_LOCAL:
-			cgit_log_link(buf, NULL, "branch-deco", buf, NULL, ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg, 0);
+			cgit_log_link(buf, NULL, "branch-deco", buf, NULL, ctx.qry.vpath, 0, NULL, NULL,
+				ctx.qry.showmsg, 0);
 			break;
 		case DECORATION_REF_TAG:
 			if (!refs_read_ref(get_main_ref_store(the_repository), deco->name, &oid_tag) &&
@@ -411,7 +437,8 @@ void cgit_print_commit_decorations(struct commit *commit)
 			);
 			break;
 		default:
-			cgit_commit_link(buf, NULL, "deco", ctx.qry.head, oid_to_hex(&commit->object.oid), ctx.qry.vpath);
+			cgit_commit_link(buf, NULL, "deco", ctx.qry.head, oid_to_hex(&commit->object.oid),
+				ctx.qry.vpath);
 			break;
 		}
 		deco = deco->next;
@@ -435,11 +462,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	if (!tip)
 		tip = ctx.qry.head;
 	tip = disambiguate_ref(tip, &must_free_tip);
-	if (tip && tip[0] == '-') {
-		// setup_revisions() reads a leading-dash argument as an
-		// option, so a tip like "--output=<path>" would become a
-		// request to write an arbitrary file. No valid ref or object
-		// name begins with a dash, so refuse it.
+	// Checked here as well as in prepare_repo_cmd, since a caller can pass
+	// a tip of its own and setup_revisions reads a leading dash as an
+	// option.
+	if (tip && !cgit_valid_rev(tip)) {
 		cgit_print_error_page(400, "Bad Request", "Invalid revision");
 		if (must_free_tip)
 			free((char *)tip);
@@ -448,6 +474,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	}
 	strvec_push(&rev_argv, tip);
 
+	// A leading colon makes git read the path as a pathspec with magic,
+	// which can end the request inside git.
+	if (path && path[0] == ':') {
+		cgit_print_error_page(400, "Bad Request", "Invalid path");
+		if (must_free_tip)
+			free((char *)tip);
+		strvec_clear(&rev_argv);
+		return;
+	}
+
 	if (grep && pattern && *pattern) {
 		pattern = xstrdup(pattern);
 		if (!strcmp(grep, "grep") || !strcmp(grep, "author") || !strcmp(grep, "committer")) {
@@ -455,16 +491,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		} else if (!strcmp(grep, "range")) {
 			char *arg;
 
-			// Each whitespace separated token is taken as a
-			// revision expression only, since a leading dash
-			// would reach setup_revisions as a rev-list option.
-			// The tip pushed above goes away, since the range
-			// supersedes it.
+			// Each whitespace separated token has to pass the same
+			// check as any other revision. The tip pushed above goes
+			// away, since the range supersedes it.
 			strvec_pop(&rev_argv);
 			while ((arg = next_token(&pattern))) {
-				if (*arg == '-') {
-					fprintf(stderr, "[cgit] Bad range expression: %s\n", arg);
-					break;
+				if (!valid_range_token(arg)) {
+					cgit_print_error_page(400, "Bad Request", "Invalid revision");
+					if (must_free_tip)
+						free((char *)tip);
+					strvec_clear(&rev_argv);
+					return;
 				}
 				strvec_push(&rev_argv, arg);
 			}
@@ -504,6 +541,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	load_ref_decorations(NULL, DECORATE_FULL_REFS);
 	rev.show_decorations = 1;
 	rev.grep_filter.ignore_case = 1;
+	// The search is literal. A pattern would run as a regular expression
+	// over every message in the history, and one with a backreference
+	// takes exponential time to match.
+	rev.grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
 
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = ctx.cfg.renamelimit;
@@ -511,7 +552,13 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
 
 	compile_grep_patterns(&rev.grep_filter);
-	prepare_revision_walk(&rev);
+	// A failed setup leaves the walk holding freed commits, so it must
+	// not be read from.
+	if (prepare_revision_walk(&rev)) {
+		cgit_print_error_page(500, "Internal Server Error", "Unable to read the history");
+		strvec_clear(&rev_argv);
+		return;
+	}
 
 	if (pager) {
 		cgit_print_layout_start();
@@ -549,14 +596,14 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	if (ofs < 0)
 		ofs = 0;
 
-	for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; ) {
+	for (i = 0; i < ofs && (commit = get_revision(&rev)); ) {
 		if (should_show(commit, &rev))
 			i++;
 		release_commit_memory(the_repository->parsed_objects, commit);
 		commit->parents = NULL;
 	}
 
-	for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; ) {
+	for (i = 0; i < cnt && (commit = get_revision(&rev)); ) {
 		// Clearing the flag per commit keeps a commit from being
 		// diffed twice when the file or line columns are on.
 		counts_ready = 0;
@@ -570,7 +617,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	if (pager) {
 		print_pager(&rev, ofs, cnt);
 		cgit_print_layout_end();
-	} else if ((commit = get_revision(&rev)) != NULL) {
+	} else if ((commit = get_revision(&rev))) {
 		htmlf("<tr class='nohover'><td colspan='%d'>", columns);
 		cgit_log_link(
 			"[...]", NULL, NULL, ctx.qry.head, NULL, ctx.qry.vpath, 0,
@@ -579,8 +626,6 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		html("</td></tr>\n");
 	}
 
-	// The cast is safe because must_free_tip is only set for a string this
-	// function allocated.
 	if (must_free_tip)
 		free((char *)tip);
 }