Harden the page renderers

1 file changed, 39 insertions, 22 deletions

diff --git a/source/ui-tree.c b/source/ui-tree.cindex 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. 
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.