Harden the page renderers

1 file changed, 39 insertions, 22 deletions

diff --git a/source/ui-tree.c b/source/ui-tree.c
index aab448e..4d829fe 100644
--- a/source/ui-tree.c
+++ b/source/ui-tree.c
@@ -20,6 +20,9 @@
 #define HEXDUMP_ROW_HALF (HEXDUMP_ROW_BYTES / 2)
 #define HEXDUMP_GAP_WIDTH 4
 
+// How many single-child directories a listing row follows into one path.
+#define MAX_DIR_CHAIN_LEVELS 15
+
 enum walk_state {
 	WALK_LOOKING,
 	WALK_LISTING,
@@ -41,10 +44,8 @@ struct walk_tree_context {
 	size_t entries_nr, entries_alloc;
 };
 
-/*
- * The count runs past one as soon as a second entry or a file turns up, which
- * ends the descent.
- */
+// The count runs past one as soon as a second entry or a file turns up, which
+// ends the descent.
 struct only_child {
 	struct strbuf *path;
 	struct object_id oid;
@@ -56,20 +57,24 @@ struct only_child {
  * The anchors are handed over in batches rather than a write per line, and
  * never built whole, which would come to several times the blob's size.
  */
+static void add_linenumber(struct strbuf *numbers, unsigned long lineno)
+{
+	strbuf_addf(numbers, "<a id='n%lu' href='#n%lu'>%lu</a>\n", lineno, lineno, lineno);
+}
+
 static void print_linenumbers(const char *buf, unsigned long size)
 {
-	const char *numberfmt = "<a id='n%1$d' href='#n%1$d'>%1$d</a>\n";
 	struct strbuf numbers = STRBUF_INIT;
 	unsigned long lineno = 0, idx = 0;
 
 	if (size) {
-		strbuf_addf(&numbers, numberfmt, ++lineno);
+		add_linenumber(&numbers, ++lineno);
 
 		// The newline that ends the last line must not open a line of
 		// its own, so the final byte is left out of the scan.
 		while (idx < size - 1) {
 			if (buf[idx] == '\n') {
-				strbuf_addf(&numbers, numberfmt, ++lineno);
+				add_linenumber(&numbers, ++lineno);
 				if (numbers.len >= HTML_BATCH) {
 					html_raw(numbers.buf, numbers.len);
 					strbuf_reset(&numbers);
@@ -136,7 +141,7 @@ static void print_binary_buffer(char *buf, unsigned long size)
 		for (idx = 0; idx < HEXDUMP_ROW_BYTES && offset + idx < size; idx++) {
 			int gap = idx == HEXDUMP_ROW_HALF ? HEXDUMP_GAP_WIDTH : 1;
 
-			strbuf_addf(&row, "%*s%02x", gap, "", buf[idx] & 0xff);
+			strbuf_addf(&row, "%*s%02x", gap, "", (unsigned char)buf[idx]);
 		}
 		strbuf_addstr(&row, " </td><td class='hex'>");
 		html_raw(row.buf, row.len);
@@ -166,7 +171,7 @@ static bool print_object(const struct object_id *oid, const char *path,
 
 	type = odb_read_object_info(the_repository->objects, oid, &size);
 	if (type == OBJ_BAD) {
-		cgit_print_error_page(404, "Not Found", "Bad object name: %s", oid_to_hex(oid));
+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", oid_to_hex(oid));
 		return false;
 	}
 
@@ -180,7 +185,8 @@ static bool print_object(const struct object_id *oid, const char *path,
 
 	buf = odb_read_object(the_repository->objects, oid, &type, &size);
 	if (!buf) {
-		cgit_print_error_page(500, "Internal Server Error", "Error reading object %s", oid_to_hex(oid));
+		cgit_print_error_page(500, "Internal Server Error", "Unable to read object %s",
+			oid_to_hex(oid));
 		return false;
 	}
 	// buffer_is_binary only sniffs the front of the blob, and a NUL past
@@ -239,10 +245,13 @@ static void print_dir_chain(const struct object_id *oid, char *name, char *rev,
 	struct pathspec paths = {
 		.nr = 0
 	};
+	int levels = 0;
 
 	oidcpy(&child.oid, oid);
 
-	while (child.count == 1) {
+	// Each level links the whole path so far, so a chain of thousands of
+	// single directories would make one row quadratic in size.
+	while (child.count == 1 && levels++ < MAX_DIR_CHAIN_LEVELS) {
 		cgit_tree_link(name, NULL, "ls-dir", ctx.qry.head, rev, fullpath->buf);
 
 		tree = lookup_tree(the_repository, &child.oid);
@@ -305,17 +314,22 @@ static void print_ls_row(const struct object_id *oid, const char *pathname,
 			strbuf_addf(&class, " %s", ext + 1);
 		cgit_tree_link(name, NULL, class.buf, ctx.qry.head, walk->rev, fullpath.buf);
 	}
-	if (S_ISLNK(mode)) {
+	// A target longer than any path is no link, and reading it would pull
+	// a blob of any size into memory.
+	if (S_ISLNK(mode) && size <= PATH_MAX) {
 		html(" -> ");
 		buf = odb_read_object(the_repository->objects, oid, &type, &size);
 		if (!buf) {
-			htmlf("Error reading object: %s", oid_to_hex(oid));
+			htmlf("Unable to read object %s", oid_to_hex(oid));
 			goto cleanup;
 		}
 		strbuf_addbuf(&linkpath, &fullpath);
 		strbuf_addf(&linkpath, "/../%s", buf);
-		strbuf_normalize_path(&linkpath);
-		cgit_tree_link(buf, NULL, class.buf, ctx.qry.head, walk->rev, linkpath.buf);
+		// A target climbing above the tree root has nothing to link to.
+		if (strbuf_normalize_path(&linkpath))
+			html_txt(buf);
+		else
+			cgit_tree_link(buf, NULL, class.buf, ctx.qry.head, walk->rev, linkpath.buf);
 		free(buf);
 		strbuf_release(&linkpath);
 	}
@@ -455,8 +469,10 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 	return 0;
 }
 
-// Either argument may be null, in which case the head of the current query
-// stands in for the revision and the listing starts at the root of the tree.
+/*
+ * Either argument may be null, in which case the head of the current query
+ * stands in for the revision and the listing starts at the root of the tree.
+ */
 void cgit_print_tree(const char *rev, char *path)
 {
 	struct object_id oid;
@@ -482,18 +498,18 @@ void cgit_print_tree(const char *rev, char *path)
 		rev = ctx.qry.head;
 
 	if (repo_get_oid(the_repository, rev, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Invalid revision name: %s", rev);
+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);
 		return;
 	}
 	commit = lookup_commit_reference(the_repository, &oid);
 	if (!commit || repo_parse_commit(the_repository, commit)) {
-		cgit_print_error_page(404, "Not Found", "Invalid commit reference: %s", rev);
+		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);
 		return;
 	}
 
 	walk.rev = xstrdup(rev);
 
-	if (path == NULL) {
+	if (!path) {
 		ls_tree(get_commit_tree_oid(commit), &walk);
 		goto cleanup;
 	}
@@ -502,10 +518,11 @@ void cgit_print_tree(const char *rev, char *path)
 	if (walk.state == WALK_LISTING) {
 		ls_flush(&walk);
 		ls_tail();
-	} else if (walk.state == WALK_BLOB_SHOWN)
+	} else if (walk.state == WALK_BLOB_SHOWN) {
 		cgit_print_layout_end();
-	else if (walk.state == WALK_LOOKING)
+	} else if (walk.state == WALK_LOOKING) {
 		cgit_print_error_page(404, "Not Found", "Path not found");
+	}
 	// WALK_ERROR_SHOWN is left alone, since the error page print_object
 	// put out is already complete.