Answer 404 for any path below the about page

The about page served every file beside the readme as a page of its
own, through the about filter, and answered 200 for directories and
missing files alike. Images it passed through came from the root of the
default branch whatever the readme's own directory and ref. A readme on
disk exposed its whole directory this way.

6 files changed, 44 insertions, 135 deletions

diff --git a/MANUAL.txt b/MANUAL.txtindex a06ef7f..9f6c6f1 100644--- a/MANUAL.txt+++ b/MANUAL.txt@@ -705,10 +705,7 @@ repo.readme:: 	or by hash may be prepended, followed by a colon, as in 	"master:docs/readme.mkd". If the value begins with a colon, as in 	":docs/readme.rst", the head given in the query or the default branch of-	the repository is used. Sharing any file exposes that entire directory-	tree to the "/about/PATH" endpoints, so be sure that there are no-	non-public files located in the same directory as the readme file.-	Default value: <readme>.+	the repository is used. Default value: <readme>.  repo.section:: 	Override the current section name for this repository. Default value:
diff --git a/MANUAL.txt b/MANUAL.txt
index a06ef7f..9f6c6f1 100644
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -705,10 +705,7 @@ repo.readme::
 	or by hash may be prepended, followed by a colon, as in
 	"master:docs/readme.mkd". If the value begins with a colon, as in
 	":docs/readme.rst", the head given in the query or the default branch of
-	the repository is used. Sharing any file exposes that entire directory
-	tree to the "/about/PATH" endpoints, so be sure that there are no
-	non-public files located in the same directory as the readme file.
-	Default value: <readme>.
+	the repository is used. Default value: <readme>.
 
 repo.section::
 	Override the current section name for this repository. Default value:
diff --git a/source/cmd.c b/source/cmd.cindex 49c1eb4..08de123 100644--- a/source/cmd.c+++ b/source/cmd.c@@ -34,39 +34,28 @@ static void head_fn(void)  static void about_fn(void) {-	char *currenturl, *query, *redirect;-	size_t path_info_len;+	char *query, *summary; +	// The page shows the readme alone, and the readme's links lead into+	// the tree, so nothing lives below it.+	if (ctx.qry.path) {+		cgit_print_error_page(404, "Not Found", "Not found");+		return;+	} 	if (!ctx.repo) { 		cgit_print_site_readme(); 		return; 	}--	// The about page resolves relative links against its own URL, so it-	// only works with a trailing slash. The branch and the rest of the-	// query travel along with either redirect.-	path_info_len = ctx.env.path_info ? strlen(ctx.env.path_info) : 0;-	if (!ctx.qry.path &&-		(!ctx.qry.url || !*ctx.qry.url || ctx.qry.url[strlen(ctx.qry.url) - 1] != '/') &&-		(!path_info_len || ctx.env.path_info[path_info_len - 1] != '/')) {-		currenturl = cgit_currenturl();-		query = cgit_currentquery();-		redirect = cgit_fmtalloc("%s/", currenturl);-		cgit_redirect(redirect, query, true);-		free(currenturl);-		free(query);-		free(redirect);-	} else if (ctx.repo->readme.nr) {-		cgit_print_repo_readme(ctx.qry.path);-	} else {-		currenturl = cgit_currenturl();-		query = cgit_currentquery();-		redirect = cgit_fmtalloc("%s../", currenturl);-		cgit_redirect(redirect, query, false);-		free(currenturl);-		free(query);-		free(redirect);+	if (ctx.repo->readme.nr) {+		cgit_print_repo_readme();+		return; 	}+	// The branch and the rest of the query travel along to the summary.+	query = cgit_currentquery();+	summary = cgit_repourl(ctx.repo->url);+	cgit_redirect(summary, query, false);+	free(query);+	free(summary); }  static void atom_fn(void)
diff --git a/source/cmd.c b/source/cmd.c
index 49c1eb4..08de123 100644
--- a/source/cmd.c
+++ b/source/cmd.c
@@ -34,39 +34,28 @@ static void head_fn(void)
 
 static void about_fn(void)
 {
-	char *currenturl, *query, *redirect;
-	size_t path_info_len;
+	char *query, *summary;
 
+	// The page shows the readme alone, and the readme's links lead into
+	// the tree, so nothing lives below it.
+	if (ctx.qry.path) {
+		cgit_print_error_page(404, "Not Found", "Not found");
+		return;
+	}
 	if (!ctx.repo) {
 		cgit_print_site_readme();
 		return;
 	}
-
-	// The about page resolves relative links against its own URL, so it
-	// only works with a trailing slash. The branch and the rest of the
-	// query travel along with either redirect.
-	path_info_len = ctx.env.path_info ? strlen(ctx.env.path_info) : 0;
-	if (!ctx.qry.path &&
-		(!ctx.qry.url || !*ctx.qry.url || ctx.qry.url[strlen(ctx.qry.url) - 1] != '/') &&
-		(!path_info_len || ctx.env.path_info[path_info_len - 1] != '/')) {
-		currenturl = cgit_currenturl();
-		query = cgit_currentquery();
-		redirect = cgit_fmtalloc("%s/", currenturl);
-		cgit_redirect(redirect, query, true);
-		free(currenturl);
-		free(query);
-		free(redirect);
-	} else if (ctx.repo->readme.nr) {
-		cgit_print_repo_readme(ctx.qry.path);
-	} else {
-		currenturl = cgit_currenturl();
-		query = cgit_currentquery();
-		redirect = cgit_fmtalloc("%s../", currenturl);
-		cgit_redirect(redirect, query, false);
-		free(currenturl);
-		free(query);
-		free(redirect);
+	if (ctx.repo->readme.nr) {
+		cgit_print_repo_readme();
+		return;
 	}
+	// The branch and the rest of the query travel along to the summary.
+	query = cgit_currentquery();
+	summary = cgit_repourl(ctx.repo->url);
+	cgit_redirect(summary, query, false);
+	free(query);
+	free(summary);
 }
 
 static void atom_fn(void)
diff --git a/source/ui-summary.c b/source/ui-summary.cindex 61cd6ac..e7b229a 100644--- a/source/ui-summary.c+++ b/source/ui-summary.c@@ -4,8 +4,7 @@  * with the clone urls, reusing the listings ui-refs.c and ui-log.c draw  * elsewhere. The about page renders the readme that config parsing picked for  * the repository, either through the configured about filter or escaped as- * plain text, and it can serve a file sitting beside that readme so links- * inside the readme resolve.+ * plain text.  */  #include "cgit.h"@@ -14,7 +13,6 @@ #include "shared.h" #include "ui-blob.h" #include "ui-log.h"-#include "ui-plain.h" #include "ui-refs.h" #include "ui-shared.h" #include "ui-summary.h"@@ -52,66 +50,6 @@ static void print_clone_url(const char *url) 	cgit_print_clone_row(url, columns); } -/*- * Without the separator boundary a sibling directory that merely shares the- * base as a name prefix, such as repo.git-backup beside repo.git, would pass.- */-static int path_within(const char *base, const char *path)-{-	size_t len = strlen(base);--	return starts_with(path, base) && (path[len] == '\0' || path[len] == '/');-}--/*- * Returns a path the caller must free, or NULL when the request cannot be- * served. A null ref means the readme is a file on the server's disk, and- * such a readme is confined to its own directory, so one named without a- * directory is refused.- */-static char *resolve_about_path(const char *filename, const char *ref, const char *path)-{-	char *copy, *base_dir, *full_path;-	char *resolved_base = NULL, *resolved_full = NULL, *resolved_repo = NULL;--	// dirname is allowed to write into its argument and to return a-	// pointer into it, so base_dir borrows from a copy freed at the end.-	copy = xstrdup(filename);-	base_dir = dirname(copy);-	if (!strcmp(base_dir, ".") || !strcmp(base_dir, "..")) {-		if (!ref) {-			free(copy);-			return NULL;-		}-		full_path = xstrdup(path);-	} else {-		full_path = cgit_fmtalloc("%s/%s", base_dir, path);-	}--	// A readme in the repository directory, or above it, would open the-	// repository's own files, its config and hooks among them.-	if (!ref) {-		resolved_base = realpath(base_dir, NULL);-		resolved_full = realpath(full_path, NULL);-		resolved_repo = realpath(ctx.repo->path, NULL);-		if (-			!resolved_base || !resolved_full || !resolved_repo ||-			path_within(resolved_base, resolved_repo)          ||-			!path_within(resolved_base, resolved_full)-		) {-			free(full_path);-			full_path = NULL;-		}-	}--	free(copy);-	free(resolved_base);-	free(resolved_full);-	free(resolved_repo);--	return full_path;-}- void cgit_print_summary(void) { 	cgit_print_layout_start();@@ -130,34 +68,14 @@ void cgit_print_summary(void) 	cgit_print_layout_end(); } -void cgit_print_repo_readme(const char *path)+void cgit_print_repo_readme(void) {-	char *filename, *ref, *mimetype;-	int free_filename = 0;--	mimetype = cgit_get_mimetype_for_filename(path);-	if (mimetype && (starts_with(mimetype, "image/") || starts_with(mimetype, "video/"))) {-		ctx.page.mimetype = mimetype;-		cgit_print_plain();-		free(mimetype);-		return;-	}-	free(mimetype);+	char *filename, *ref;  	cgit_print_layout_start();-	if (ctx.repo->readme.nr == 0)-		goto done;- 	filename = ctx.repo->readme.items[0].string; 	ref = ctx.repo->readme.items[0].util; -	if (path) {-		free_filename = 1;-		filename = resolve_about_path(filename, ref, path);-		if (!filename)-			goto done;-	}- 	html("<div id='about'>"); 	if (!ctx.repo->about_filter) { 		// With no about-filter there is nothing to turn the readme@@ -182,11 +100,6 @@ void cgit_print_repo_readme(const char *path) 			html_include(filename); 		cgit_close_filter(ctx.repo->about_filter); 	}- 	html("</div>");-	if (free_filename)-		free(filename);--done: 	cgit_print_layout_end(); }
diff --git a/source/ui-summary.c b/source/ui-summary.c
index 61cd6ac..e7b229a 100644
--- a/source/ui-summary.c
+++ b/source/ui-summary.c
@@ -4,8 +4,7 @@
  * with the clone urls, reusing the listings ui-refs.c and ui-log.c draw
  * elsewhere. The about page renders the readme that config parsing picked for
  * the repository, either through the configured about filter or escaped as
- * plain text, and it can serve a file sitting beside that readme so links
- * inside the readme resolve.
+ * plain text.
  */
 
 #include "cgit.h"
@@ -14,7 +13,6 @@
 #include "shared.h"
 #include "ui-blob.h"
 #include "ui-log.h"
-#include "ui-plain.h"
 #include "ui-refs.h"
 #include "ui-shared.h"
 #include "ui-summary.h"
@@ -52,66 +50,6 @@ static void print_clone_url(const char *url)
 	cgit_print_clone_row(url, columns);
 }
 
-/*
- * Without the separator boundary a sibling directory that merely shares the
- * base as a name prefix, such as repo.git-backup beside repo.git, would pass.
- */
-static int path_within(const char *base, const char *path)
-{
-	size_t len = strlen(base);
-
-	return starts_with(path, base) && (path[len] == '\0' || path[len] == '/');
-}
-
-/*
- * Returns a path the caller must free, or NULL when the request cannot be
- * served. A null ref means the readme is a file on the server's disk, and
- * such a readme is confined to its own directory, so one named without a
- * directory is refused.
- */
-static char *resolve_about_path(const char *filename, const char *ref, const char *path)
-{
-	char *copy, *base_dir, *full_path;
-	char *resolved_base = NULL, *resolved_full = NULL, *resolved_repo = NULL;
-
-	// dirname is allowed to write into its argument and to return a
-	// pointer into it, so base_dir borrows from a copy freed at the end.
-	copy = xstrdup(filename);
-	base_dir = dirname(copy);
-	if (!strcmp(base_dir, ".") || !strcmp(base_dir, "..")) {
-		if (!ref) {
-			free(copy);
-			return NULL;
-		}
-		full_path = xstrdup(path);
-	} else {
-		full_path = cgit_fmtalloc("%s/%s", base_dir, path);
-	}
-
-	// A readme in the repository directory, or above it, would open the
-	// repository's own files, its config and hooks among them.
-	if (!ref) {
-		resolved_base = realpath(base_dir, NULL);
-		resolved_full = realpath(full_path, NULL);
-		resolved_repo = realpath(ctx.repo->path, NULL);
-		if (
-			!resolved_base || !resolved_full || !resolved_repo ||
-			path_within(resolved_base, resolved_repo)          ||
-			!path_within(resolved_base, resolved_full)
-		) {
-			free(full_path);
-			full_path = NULL;
-		}
-	}
-
-	free(copy);
-	free(resolved_base);
-	free(resolved_full);
-	free(resolved_repo);
-
-	return full_path;
-}
-
 void cgit_print_summary(void)
 {
 	cgit_print_layout_start();
@@ -130,34 +68,14 @@ void cgit_print_summary(void)
 	cgit_print_layout_end();
 }
 
-void cgit_print_repo_readme(const char *path)
+void cgit_print_repo_readme(void)
 {
-	char *filename, *ref, *mimetype;
-	int free_filename = 0;
-
-	mimetype = cgit_get_mimetype_for_filename(path);
-	if (mimetype && (starts_with(mimetype, "image/") || starts_with(mimetype, "video/"))) {
-		ctx.page.mimetype = mimetype;
-		cgit_print_plain();
-		free(mimetype);
-		return;
-	}
-	free(mimetype);
+	char *filename, *ref;
 
 	cgit_print_layout_start();
-	if (ctx.repo->readme.nr == 0)
-		goto done;
-
 	filename = ctx.repo->readme.items[0].string;
 	ref = ctx.repo->readme.items[0].util;
 
-	if (path) {
-		free_filename = 1;
-		filename = resolve_about_path(filename, ref, path);
-		if (!filename)
-			goto done;
-	}
-
 	html("<div id='about'>");
 	if (!ctx.repo->about_filter) {
 		// With no about-filter there is nothing to turn the readme
@@ -182,11 +100,6 @@ void cgit_print_repo_readme(const char *path)
 			html_include(filename);
 		cgit_close_filter(ctx.repo->about_filter);
 	}
-
 	html("</div>");
-	if (free_filename)
-		free(filename);
-
-done:
 	cgit_print_layout_end();
 }
diff --git a/source/ui-summary.h b/source/ui-summary.hindex 0f61da5..ecb1a5c 100644--- a/source/ui-summary.h+++ b/source/ui-summary.h@@ -1,14 +1,13 @@ /*  * The summary and about pages, the two a repository opens with, one listing  * its refs and recent commits and the other built from its readme. cmd.c- * reaches both, and it hands the about page whatever path follows the page- * name in the url, so a readme can link to a file beside it.+ * reaches both.  */  #ifndef CGIT_UI_SUMMARY_H #define CGIT_UI_SUMMARY_H  extern void cgit_print_summary(void);-extern void cgit_print_repo_readme(const char *path);+extern void cgit_print_repo_readme(void);  #endif // CGIT_UI_SUMMARY_H
diff --git a/source/ui-summary.h b/source/ui-summary.h
index 0f61da5..ecb1a5c 100644
--- a/source/ui-summary.h
+++ b/source/ui-summary.h
@@ -1,14 +1,13 @@
 /*
  * The summary and about pages, the two a repository opens with, one listing
  * its refs and recent commits and the other built from its readme. cmd.c
- * reaches both, and it hands the about page whatever path follows the page
- * name in the url, so a readme can link to a file beside it.
+ * reaches both.
  */
 
 #ifndef CGIT_UI_SUMMARY_H
 #define CGIT_UI_SUMMARY_H
 
 extern void cgit_print_summary(void);
-extern void cgit_print_repo_readme(const char *path);
+extern void cgit_print_repo_readme(void);
 
 #endif // CGIT_UI_SUMMARY_H
diff --git a/tests/t0301-security.sh b/tests/t0301-security.shindex 1a5ee44..ce2b5da 100755--- a/tests/t0301-security.sh+++ b/tests/t0301-security.sh@@ -445,7 +445,7 @@ test_expect_success 'the dumb transport refuses a path that climbs out' '  test_expect_success 'the about page serves nothing above the readme directory' ' 	cveq "url=cve/about/../cverc" >tmp &&-	grep "^Status: 200" tmp &&+	grep "^Status: 404" tmp && 	! grep "virtual-root" tmp && 	cveq "url=cve/about/" >tmp && 	grep "readme text" tmp
diff --git a/tests/t0301-security.sh b/tests/t0301-security.sh
index 1a5ee44..ce2b5da 100755
--- a/tests/t0301-security.sh
+++ b/tests/t0301-security.sh
@@ -445,7 +445,7 @@ test_expect_success 'the dumb transport refuses a path that climbs out' '
 
 test_expect_success 'the about page serves nothing above the readme directory' '
 	cveq "url=cve/about/../cverc" >tmp &&
-	grep "^Status: 200" tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "virtual-root" tmp &&
 	cveq "url=cve/about/" >tmp &&
 	grep "readme text" tmp
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.shindex 87a85ce..7b037bf 100755--- a/tests/t0303-robustness.sh+++ b/tests/t0303-robustness.sh@@ -218,7 +218,7 @@ test_expect_success 'a submodule below the about path does not end the request' 		echo "repo.readme=master:file-1" 	} >aboutrc && 	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/sub" cgit >tmp 2>err &&-	grep "^Status:" tmp &&+	grep "^Status: 404" tmp && 	! grep "fatal" err ' @@ -525,6 +525,7 @@ test_expect_success 'a disk readme in the repository directory serves no sibling 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/" cgit >tmp && 	grep "hello" tmp && 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/config" cgit >tmp &&+	grep "^Status: 404" tmp && 	! grep "repositoryformatversion" tmp ' @@ -617,17 +618,27 @@ test_expect_success 'pages behind an auth filter are marked private' ' 	! grep "^Vary" tmp ' -# The about page redirects to its trailing-slash form so relative links-# resolve, and the branch asked for has to survive that hop, as does the-# hop back to the summary of a repository without a readme.-test_expect_success 'the about redirects keep the query string' '+# A repository without a readme sends its about page to the summary, and+# the branch asked for has to survive that hop.+test_expect_success 'the about redirect keeps the query string' ' 	git -C repos/rob branch other HEAD &&-	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other&x=1" cgit >tmp &&-	grep "^Status: 301" tmp &&-	grep "^Location: /rob/about/?h=other&x=1$" tmp &&-	robq "url=rob/about/&h=other" >tmp &&+	robq "url=rob/about&h=other&x=1" >tmp && 	grep "^Status: 302" tmp &&-	grep "^Location: /rob/about/../?h=other$" tmp+	grep "^Location: /rob/?h=other&x=1$" tmp &&+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other" cgit >tmp &&+	grep "^Status: 200" tmp+'++# The about page shows the readme alone and its links lead into the tree,+# so a path below it names nothing, the site about page included.+test_expect_success 'a path below the about page answers 404' '+	for path in file-1 sub/ ../robrc+	do+		CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/$path" cgit >tmp &&+		grep "^Status: 404" tmp || return 1+	done &&+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="p=about&path=x" cgit >tmp &&+	grep "^Status: 404" tmp '  test_expect_success 'a symlink whose target is a large blob is listed without it' '
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.sh
index 87a85ce..7b037bf 100755
--- a/tests/t0303-robustness.sh
+++ b/tests/t0303-robustness.sh
@@ -218,7 +218,7 @@ test_expect_success 'a submodule below the about path does not end the request'
 		echo "repo.readme=master:file-1"
 	} >aboutrc &&
 	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/sub" cgit >tmp 2>err &&
-	grep "^Status:" tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "fatal" err
 '
 
@@ -525,6 +525,7 @@ test_expect_success 'a disk readme in the repository directory serves no sibling
 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/" cgit >tmp &&
 	grep "hello" tmp &&
 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/config" cgit >tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "repositoryformatversion" tmp
 '
 
@@ -617,17 +618,27 @@ test_expect_success 'pages behind an auth filter are marked private' '
 	! grep "^Vary" tmp
 '
 
-# The about page redirects to its trailing-slash form so relative links
-# resolve, and the branch asked for has to survive that hop, as does the
-# hop back to the summary of a repository without a readme.
-test_expect_success 'the about redirects keep the query string' '
+# A repository without a readme sends its about page to the summary, and
+# the branch asked for has to survive that hop.
+test_expect_success 'the about redirect keeps the query string' '
 	git -C repos/rob branch other HEAD &&
-	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other&x=1" cgit >tmp &&
-	grep "^Status: 301" tmp &&
-	grep "^Location: /rob/about/?h=other&x=1$" tmp &&
-	robq "url=rob/about/&h=other" >tmp &&
+	robq "url=rob/about&h=other&x=1" >tmp &&
 	grep "^Status: 302" tmp &&
-	grep "^Location: /rob/about/../?h=other$" tmp
+	grep "^Location: /rob/?h=other&x=1$" tmp &&
+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other" cgit >tmp &&
+	grep "^Status: 200" tmp
+'
+
+# The about page shows the readme alone and its links lead into the tree,
+# so a path below it names nothing, the site about page included.
+test_expect_success 'a path below the about page answers 404' '
+	for path in file-1 sub/ ../robrc
+	do
+		CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/$path" cgit >tmp &&
+		grep "^Status: 404" tmp || return 1
+	done &&
+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="p=about&path=x" cgit >tmp &&
+	grep "^Status: 404" tmp
 '
 
 test_expect_success 'a symlink whose target is a large blob is listed without it' '