"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
gotwebd request path validation for -portable
To:
gameoftrees@openbsd.org
Date:
Mon, 8 Sep 2025 14:07:48 +0200

Download raw body.

Thread
Block gotwebd requests with paths that point at repositories outside
the server's assigned repository directory. This is needed for -portable
where we cannot assume that chroot or unveil would prevent such access.

There is no problem on OpenBSD because unveil(2) already prevents this.

At present this is an accidental leak of unrelated repositories which
a system running gotwebd might have somewhere on disk where the _gotwebd
user can read them. Once we add authentication to gotwebd this bug could
potentially be used to bypass authentication.

This fix applies early defenses in the query parser and when gotwebd looks
up repositories on disk in case the query parser's verification is bypassed
somehow. Is this good enough?

ok?

(Tehnically the HEADREF is not a path and could have much stricter checks
applied to it. But got_ref_open will already do those additional checks.)


M  gotwebd/fcgi.c    |  30+  0-
M  gotwebd/gotweb.c  |  33+  0-

2 files changed, 63 insertions(+), 0 deletions(-)

commit - c3d3a336ffa1b625a5ddcff77bd6aaec86638279
commit + 1f916215fba3e05332679929a5f0736fc6628e73
blob - beabb467f8b0a218c66f84aa75b3a377276a55c2
blob + 30774393e8a5bd14695eeda37cf4105e6c11f907
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -260,6 +260,24 @@ urldecode(char *url)
 }
 
 static const struct got_error *
+validate_path(const char *path)
+{
+	size_t len = strlen(path);
+
+	if (len >= 3 && strncmp(path, "../", 3) == 0)
+		return got_error(GOT_ERR_BAD_QUERYSTRING);
+
+	if (len >= 4 && strstr(path, "/../") != NULL)
+		return got_error(GOT_ERR_BAD_QUERYSTRING);
+
+	if (len >= 3 && strcmp(path + len - 3, "/..") == 0)
+		return got_error(GOT_ERR_BAD_QUERYSTRING);
+
+	return NULL;
+
+}
+
+static const struct got_error *
 assign_querystring(struct querystring *qs, char *key, char *value)
 {
 	const struct got_error *error = NULL;
@@ -294,6 +312,9 @@ assign_querystring(struct querystring *qs, char *key, 
 			}
 			break;
 		case RFILE:
+			error = validate_path(value);
+			if (error)
+				goto done;
 			if (strlcpy(qs->file, value, sizeof(qs->file)) >=
 			    sizeof(qs->file)) {
 				error = got_error_msg(GOT_ERR_NO_SPACE,
@@ -302,6 +323,9 @@ assign_querystring(struct querystring *qs, char *key, 
 			}
 			break;
 		case FOLDER:
+			error = validate_path(value);
+			if (error)
+				goto done;
 			if (strlcpy(qs->folder, value[0] ? value : "/",
 			    sizeof(qs->folder)) >= sizeof(qs->folder)) {
 				error = got_error_msg(GOT_ERR_NO_SPACE,
@@ -311,6 +335,9 @@ assign_querystring(struct querystring *qs, char *key, 
 			}
 			break;
 		case HEADREF:
+			error = validate_path(value);
+			if (error)
+				goto done;
 			if (strlcpy(qs->headref, value, sizeof(qs->headref)) >=
 			    sizeof(qs->headref)) {
 				error = got_error_msg(GOT_ERR_NO_SPACE,
@@ -329,6 +356,9 @@ assign_querystring(struct querystring *qs, char *key, 
 			}
 			break;
 		case PATH:
+			error = validate_path(value);
+			if (error)
+				goto done;
 			if (strlcpy(qs->path, value, sizeof(qs->path)) >=
 			    sizeof(qs->path)) {
 				error = got_error_msg(GOT_ERR_NO_SPACE,
blob - cdd5b253e75c190fd7f3c710e9eadeefea473c8a
blob + d4eecc1c2d5aecc314dd3ab30ca5cf784947b3fc
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -929,7 +929,34 @@ gotweb_render_absolute_url(struct request *c, struct g
 	return gotweb_render_url(c, url);
 }
 
+/* 
+ * Ensure that a path sent in the request will not escape from the given
+ * parent directory. This matters for got-portable where we are not
+ * necessarily running in chroot and cannot be protected by unveil(2).
+ */
 static const struct got_error *
+validate_path(const char *path, const char *parent_path,
+    const char *orig_path)
+{
+	const struct got_error *error = NULL;
+	char *abspath;
+
+	abspath = realpath(path, NULL);
+	if (abspath == NULL) {
+		/* Paths which cannot be resolved are safe. */
+		if (errno == ENOENT || errno == EACCES || errno == ENOTDIR)
+			return NULL;
+		return got_error_from_errno("realpath");
+	}
+
+	if (!got_path_is_child(abspath, parent_path, strlen(parent_path)))
+		error = got_error_path(orig_path, GOT_ERR_NOT_GIT_REPO);
+
+	free(abspath);
+	return error;
+}
+
+static const struct got_error *
 gotweb_load_got_path(struct repo_dir **rp, const char *dir,
     struct request *c)
 {
@@ -949,6 +976,12 @@ gotweb_load_got_path(struct repo_dir **rp, const char 
 	    GOTWEB_GIT_DIR) == -1)
 		return got_error_from_errno("asprintf");
 
+	error = validate_path(dir_test, srv->repos_path, dir);
+	if (error) {
+		free(dir_test);
+		return error;
+	}
+
 	dt = opendir(dir_test);
 	if (dt == NULL) {
 		free(dir_test);