[PATCH 2/2] daemon: Verify base-path and interpolated-path early

[PATCH 2/2] daemon: Verify base-path and interpolated-path early

[Johannes Sixt]

Any request to the daemon would fail if either interpolated-path or
base-path (if specified) would not be absolute. Hence, we can check those
paths for validity upfront and not start the daemon at all if the paths are
invalid.

Additionally, we now check that the base-path is an existing directory.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 daemon.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/daemon.c b/daemon.c
index dd0177f..64c7fff 100644
--- a/daemon.c
+++ b/daemon.c
@@ -220,12 +220,6 @@ static char *path_ok(struct interp *itable)
 		}
 	}
 	else if (interpolated_path && saw_extended_args) {
-		if (*dir != '/') {
-			/* Allow only absolute */
-			logerror("'%s': Non-absolute path denied (interpolated-path active)", dir);
-			return NULL;
-		}
-
 		interpolate(interp_path, PATH_MAX, interpolated_path,
 			    interp_table, ARRAY_SIZE(interp_table));
 		loginfo("Interpolated dir '%s'", interp_path);
@@ -233,11 +227,6 @@ static char *path_ok(struct interp *itable)
 		dir = interp_path;
 	}
 	else if (base_path) {
-		if (*dir != '/') {
-			/* Allow only absolute */
-			logerror("'%s': Non-absolute path denied (base-path active)", dir);
-			return NULL;
-		}
 		snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
 		dir = rpath;
 	}
@@ -1184,6 +1173,19 @@ int main(int argc, char **argv)
 	if (strict_paths && (!ok_paths || !*ok_paths))
 		die("option --strict-paths requires a whitelist");

+	if (base_path) {
+		struct stat st;
+
+		if (!is_absolute_path(base_path))
+			die("base-path must be absolute");
+		if (stat(base_path, &st) || !S_ISDIR(st.st_mode))
+			die("base-path '%s' does not exist or "
+			    "is not a directory", base_path);
+	}
+
+	if (interpolated_path && !is_absolute_path(interpolated_path))
+		die("interpolated-path must be absolute");
+
 	if (inetd_mode) {
 		struct sockaddr_storage ss;
 		struct sockaddr *peer = (struct sockaddr *)&ss;
-- 
1.5.4.3.229.g5c72

Re: [PATCH 2/2] daemon: Verify base-path and interpolated-path early

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Any request to the daemon would fail if either interpolated-path or
> base-path (if specified) would not be absolute. Hence, we can check those
> paths for validity upfront and not start the daemon at all if the paths are
> invalid.
>
> Additionally, we now check that the base-path is an existing directory.

Looks good.  Thanks.

Re: [PATCH 2/2] daemon: Verify base-path and interpolated-path early

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> Any request to the daemon would fail if either interpolated-path or
>> base-path (if specified) would not be absolute. Hence, we can check those
>> paths for validity upfront and not start the daemon at all if the paths are
>> invalid.
>>
>> Additionally, we now check that the base-path is an existing directory.
> 
> Looks good.  Thanks.

I just discovered that this patch is crap. Please drop it!

We must not remove the checks from path_ok() because they verify the
*client-supplied* part of the path, which must be absolute.

Nevertheless, it would be useful to verify that --base-path points to an
existing directory. What I actually wanted to implement is this:

-- >8 --
daemon: ensure that base-path is an existing directory

Any request to the daemon would fail if base-path (if specified) is not
a directory. We now check for this condition early.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 daemon.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/daemon.c b/daemon.c
index dd0177f..2b4a6f1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1184,6 +1184,14 @@ int main(int argc, char **argv)
 	if (strict_paths && (!ok_paths || !*ok_paths))
 		die("option --strict-paths requires a whitelist");

+	if (base_path) {
+		struct stat st;
+
+		if (stat(base_path, &st) || !S_ISDIR(st.st_mode))
+			die("base-path '%s' does not exist or "
+			    "is not a directory", base_path);
+	}
+
 	if (inetd_mode) {
 		struct sockaddr_storage ss;
 		struct sockaddr *peer = (struct sockaddr *)&ss;
-- 
1.5.4.3.231.g5d43e