Harden the request path, scan and error recovery

1 file changed, 94 insertions, 23 deletions

diff --git a/source/scan-tree.c b/source/scan-tree.c
index ba2149a..4110511 100644
--- a/source/scan-tree.c
+++ b/source/scan-tree.c
@@ -32,7 +32,7 @@ static int stat_entry(const char *dir, const char *name, struct stat *st)
 	// A missing entry is the ordinary answer for a directory that is not a
 	// repository, so only some other failure is worth reporting.
 	if (err && errno != ENOENT)
-		fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", dir, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", dir, strerror(errno), errno);
 	strbuf_release(&path);
 	return err;
 }
@@ -48,13 +48,30 @@ static int is_git_dir(const char *path)
 	return 1;
 }
 
-// A filter is a command cgit runs, and a repository's own files belong to
-// whoever can push to it, so their filter keys wait on trust-scan-filters.
-static int trusted_key(const char *name)
+/*
+ * A repository's own files belong to whoever can push to it, so the keys
+ * that run a command, put raw markup on the page, read a file off the disk
+ * or place a link wait on trust-scan-config. A readme naming a git object,
+ * the form with a colon, only reads from the repository and passes.
+ */
+static int trusted_key(const char *name, const char *value)
 {
-	if (!ends_with(name, "-filter") || ctx.cfg.trust_scan_filters)
+	int untrusted;
+
+	if (ctx.cfg.trust_scan_config)
 		return 1;
-	fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-filters is not set\n", name,
+	untrusted =
+		ends_with(name, "-filter")							||
+		!strcmp(name, "head-content")						||
+		!strcmp(name, "module-link")						||
+		starts_with(name, "module-link.")					||
+		!strcmp(name, "logo")								||
+		!strcmp(name, "logo-link")							||
+		!strcmp(name, "clone-url")							||
+		(!strcmp(name, "readme") && !strchr(value, ':'));
+	if (!untrusted)
+		return 1;
+	fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-config is not set\n", name,
 		current_repo->path);
 	return 0;
 }
@@ -64,13 +81,16 @@ static int apply_gitconfig(const char *key, const char *value,
 {
 	const char *name;
 
+	// A key with no value is legal git config and nothing here can use it.
+	if (!value)
+		return 0;
 	if (!strcmp(key, "gitweb.owner"))
 		cgit_repo_config(current_repo, "owner", value);
 	else if (!strcmp(key, "gitweb.description"))
 		cgit_repo_config(current_repo, "desc", value);
 	else if (!strcmp(key, "gitweb.category"))
 		cgit_repo_config(current_repo, "section", value);
-	else if (skip_prefix(key, "cgit.", &name) && trusted_key(name))
+	else if (skip_prefix(key, "cgit.", &name) && trusted_key(name, value))
 		cgit_repo_config(current_repo, name, value);
 
 	return 0;
@@ -78,7 +98,7 @@ static int apply_gitconfig(const char *key, const char *value,
 
 static void apply_cgitrc(const char *name, const char *value)
 {
-	if (trusted_key(name))
+	if (trusted_key(name, value))
 		cgit_repo_config(current_repo, name, value);
 }
 
@@ -135,7 +155,7 @@ static void add_repo(const char *base, struct strbuf *path)
 	size_t desc_size;
 
 	if (stat(path->buf, &st)) {
-		fprintf(stderr, "[cgit] Error accessing %s: %s (%d)\n", path->buf, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to access %s: %s (%d)\n", path->buf, strerror(errno), errno);
 		return;
 	}
 
@@ -143,8 +163,10 @@ static void add_repo(const char *base, struct strbuf *path)
 	pathlen = path->len;
 
 	if (ctx.cfg.strict_export) {
+		struct stat export_st;
+
 		strbuf_addstr(path, ctx.cfg.strict_export);
-		if (stat(path->buf, &st))
+		if (stat(path->buf, &export_st))
 			return;
 		strbuf_setlen(path, pathlen);
 	}
@@ -165,11 +187,31 @@ static void add_repo(const char *base, struct strbuf *path)
 		strbuf_setlen(&relpath, relpath.len - 1);
 	if (relpath.len >= 5 && !strcmp(relpath.buf + relpath.len - 5, "/.git"))
 		strbuf_setlen(&relpath, relpath.len - 5);
+	else if (!strcmp(relpath.buf, ".git"))
+		strbuf_setlen(&relpath, 0);
+	// The scan root may itself be the repository, leaving nothing after
+	// the base, so that one is named after its directory.
+	if (!relpath.len) {
+		const char *end = path->buf + pathlen - 1, *start;
+
+		if (end - path->buf >= 5 && !strncmp(end - 5, "/.git", 5))
+			end -= 5;
+		start = end;
+		while (start > path->buf && start[-1] != '/')
+			start--;
+		strbuf_add(&relpath, start, end - start);
+	}
 
 	current_repo = cgit_add_repo(relpath.buf);
 	if (ctx.cfg.enable_git_config) {
+		// A pusher wrote this file, so a broken one is skipped with a
+		// warning rather than allowed to end the request.
+		struct config_options opts = { .error_action = CONFIG_ERROR_SILENT };
+
 		strbuf_addstr(path, "config");
-		git_config_from_file(apply_gitconfig, path->buf, NULL);
+		if (git_config_from_file_with_options(apply_gitconfig, path->buf, NULL,
+				CONFIG_SCOPE_UNKNOWN, &opts))
+			fprintf(stderr, "[cgit] Ignoring unreadable config in %s\n", path->buf);
 		strbuf_setlen(path, pathlen);
 	}
 
@@ -181,16 +223,15 @@ static void add_repo(const char *base, struct strbuf *path)
 	}
 	current_repo->path = cgit_strdup_first_line(path->buf);
 	while (!current_repo->owner) {
-		if ((pwd = getpwuid(st.st_uid)) == NULL) {
-			fprintf(stderr, "[cgit] Error reading owner-info for %s: %s (%d)\n",
+		if (!(pwd = getpwuid(st.st_uid))) {
+			fprintf(stderr, "[cgit] Unable to read the owner of %s: %s (%d)\n",
 				path->buf, strerror(errno), errno);
 			break;
 		}
 		// A gecos field puts the owner's name in front of a comma
 		// separated list of office and phone details.
-		if (pwd->pw_gecos)
-			if ((comma = strchr(pwd->pw_gecos, ',')))
-				*comma = '\0';
+		if (pwd->pw_gecos && (comma = strchr(pwd->pw_gecos, ',')))
+			*comma = '\0';
 		current_repo->owner = cgit_strdup_first_line(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name);
 	}
 
@@ -210,7 +251,7 @@ static void add_repo(const char *base, struct strbuf *path)
 
 	strbuf_addstr(path, "cgitrc");
 	if (!stat(path->buf, &st))
-		config_file_parse(path->buf, &apply_cgitrc);
+		config_file_parse(path->buf, apply_cgitrc);
 
 	strbuf_release(&relpath);
 }
@@ -226,16 +267,45 @@ static int should_scan(const struct dirent *ent)
 	return ctx.cfg.scan_hidden_path;
 }
 
+// Symlinks are followed so that a link into the scan path counts, which means
+// a link back at an ancestor would recurse until the path ran out of room.
+// Each directory is entered once.
+static struct {
+	dev_t dev;
+	ino_t ino;
+} *visited;
+static size_t visited_nr, visited_alloc;
+
+static int already_visited(const char *path)
+{
+	struct stat st;
+	size_t i;
+
+	if (stat(path, &st))
+		return 0;
+	for (i = 0; i < visited_nr; i++)
+		if (visited[i].dev == st.st_dev && visited[i].ino == st.st_ino)
+			return 1;
+	ALLOC_GROW(visited, visited_nr + 1, visited_alloc);
+	visited[visited_nr].dev = st.st_dev;
+	visited[visited_nr].ino = st.st_ino;
+	visited_nr++;
+	return 0;
+}
+
 static void scan_path(const char *base, const char *path)
 {
-	DIR *dir = opendir(path);
+	DIR *dir;
 	struct dirent *ent;
 	struct strbuf pathbuf = STRBUF_INIT;
 	size_t pathlen = strlen(path);
 	struct stat st;
 
+	if (already_visited(path))
+		return;
+	dir = opendir(path);
 	if (!dir) {
-		fprintf(stderr, "[cgit] Error opening directory %s: %s (%d)\n", path, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", path, strerror(errno), errno);
 		return;
 	}
 
@@ -252,13 +322,14 @@ static void scan_path(const char *base, const char *path)
 	// Take in the '/' that "/.git" left in the buffer, since the loop below
 	// truncates to this length and then appends an entry name straight on.
 	pathlen++;
-	while ((ent = readdir(dir)) != NULL) {
+	while ((ent = readdir(dir))) {
 		if (!should_scan(ent))
 			continue;
 		strbuf_setlen(&pathbuf, pathlen);
 		strbuf_addstr(&pathbuf, ent->d_name);
 		if (stat(pathbuf.buf, &st)) {
-			fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", pathbuf.buf, strerror(errno), errno);
+			fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", pathbuf.buf, strerror(errno),
+				errno);
 			continue;
 		}
 		if (S_ISDIR(st.st_mode))
@@ -277,7 +348,7 @@ void scan_projects(const char *path, const char *projectsfile)
 
 	projects = fopen(projectsfile, "r");
 	if (!projects) {
-		fprintf(stderr, "[cgit] Error opening projectsfile %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to open project list %s: %s (%d)\n",
 			projectsfile, strerror(errno), errno);
 		return;
 	}
@@ -289,7 +360,7 @@ void scan_projects(const char *path, const char *projectsfile)
 		scan_path(path, line.buf);
 	}
 	if ((err = ferror(projects))) {
-		fprintf(stderr, "[cgit] Error reading from projectsfile %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to read project list %s: %s (%d)\n",
 			projectsfile, strerror(err), err);
 	}
 	fclose(projects);