{"thread":{"id":"12310","subject":"[PATCH 2/2] daemon: Verify base-path and interpolated-path early","startedAt":"2008-02-25T13:27:25Z","lastAt":"2008-02-26T12:00:55Z","messageCount":3,"participants":["Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"69883","messageId":"47C2C23D.1030609@viscovery.net","threadId":"12310","inReplyTo":null,"subject":"[PATCH 2/2] daemon: Verify base-path and interpolated-path early","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-02-25T13:27:25Z","receivedAt":"2008-02-25T13:27:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Any request to the daemon would fail if either interpolated-path or\nbase-path (if specified) would not be absolute. Hence, we can check those\npaths for validity upfront and not start the daemon at all if the paths are\ninvalid.\n\nAdditionally, we now check that the base-path is an existing directory.\n\nSigned-off-by: Johannes Sixt <johannes.sixt@telecom.at>\n---\n daemon.c |   24 +++++++++++++-----------\n 1 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex dd0177f..64c7fff 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -220,12 +220,6 @@ static char *path_ok(struct interp *itable)\n \t\t}\n \t}\n \telse if (interpolated_path && saw_extended_args) {\n-\t\tif (*dir != '/') {\n-\t\t\t/* Allow only absolute */\n-\t\t\tlogerror(\"'%s': Non-absolute path denied (interpolated-path active)\", dir);\n-\t\t\treturn NULL;\n-\t\t}\n-\n \t\tinterpolate(interp_path, PATH_MAX, interpolated_path,\n \t\t\t    interp_table, ARRAY_SIZE(interp_table));\n \t\tloginfo(\"Interpolated dir '%s'\", interp_path);\n@@ -233,11 +227,6 @@ static char *path_ok(struct interp *itable)\n \t\tdir = interp_path;\n \t}\n \telse if (base_path) {\n-\t\tif (*dir != '/') {\n-\t\t\t/* Allow only absolute */\n-\t\t\tlogerror(\"'%s': Non-absolute path denied (base-path active)\", dir);\n-\t\t\treturn NULL;\n-\t\t}\n \t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path, dir);\n \t\tdir = rpath;\n \t}\n@@ -1184,6 +1173,19 @@ int main(int argc, char **argv)\n \tif (strict_paths && (!ok_paths || !*ok_paths))\n \t\tdie(\"option --strict-paths requires a whitelist\");\n\n+\tif (base_path) {\n+\t\tstruct stat st;\n+\n+\t\tif (!is_absolute_path(base_path))\n+\t\t\tdie(\"base-path must be absolute\");\n+\t\tif (stat(base_path, &st) || !S_ISDIR(st.st_mode))\n+\t\t\tdie(\"base-path '%s' does not exist or \"\n+\t\t\t    \"is not a directory\", base_path);\n+\t}\n+\n+\tif (interpolated_path && !is_absolute_path(interpolated_path))\n+\t\tdie(\"interpolated-path must be absolute\");\n+\n \tif (inetd_mode) {\n \t\tstruct sockaddr_storage ss;\n \t\tstruct sockaddr *peer = (struct sockaddr *)&ss;\n-- \n1.5.4.3.229.g5c72\n"},{"id":"69910","messageId":"7vejb0j04z.fsf@gitster.siamese.dyndns.org","threadId":"12310","inReplyTo":"47C2C23D.1030609@viscovery.net","subject":"Re: [PATCH 2/2] daemon: Verify base-path and interpolated-path early","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-25T19:39:56Z","receivedAt":"2008-02-25T19:39:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Any request to the daemon would fail if either interpolated-path or\n> base-path (if specified) would not be absolute. Hence, we can check those\n> paths for validity upfront and not start the daemon at all if the paths are\n> invalid.\n>\n> Additionally, we now check that the base-path is an existing directory.\n\nLooks good.  Thanks.\n"},{"id":"69980","messageId":"47C3FF77.2060902@viscovery.net","threadId":"12310","inReplyTo":"7vejb0j04z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] daemon: Verify base-path and interpolated-path early","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-02-26T12:00:55Z","receivedAt":"2008-02-26T12:00:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> Johannes Sixt <j.sixt@viscovery.net> writes:\n> \n>> Any request to the daemon would fail if either interpolated-path or\n>> base-path (if specified) would not be absolute. Hence, we can check those\n>> paths for validity upfront and not start the daemon at all if the paths are\n>> invalid.\n>>\n>> Additionally, we now check that the base-path is an existing directory.\n> \n> Looks good.  Thanks.\n\nI just discovered that this patch is crap. Please drop it!\n\nWe must not remove the checks from path_ok() because they verify the\n*client-supplied* part of the path, which must be absolute.\n\nNevertheless, it would be useful to verify that --base-path points to an\nexisting directory. What I actually wanted to implement is this:\n\n-- >8 --\ndaemon: ensure that base-path is an existing directory\n\nAny request to the daemon would fail if base-path (if specified) is not\na directory. We now check for this condition early.\n\nSigned-off-by: Johannes Sixt <johannes.sixt@telecom.at>\n---\n daemon.c |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex dd0177f..2b4a6f1 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1184,6 +1184,14 @@ int main(int argc, char **argv)\n \tif (strict_paths && (!ok_paths || !*ok_paths))\n \t\tdie(\"option --strict-paths requires a whitelist\");\n\n+\tif (base_path) {\n+\t\tstruct stat st;\n+\n+\t\tif (stat(base_path, &st) || !S_ISDIR(st.st_mode))\n+\t\t\tdie(\"base-path '%s' does not exist or \"\n+\t\t\t    \"is not a directory\", base_path);\n+\t}\n+\n \tif (inetd_mode) {\n \t\tstruct sockaddr_storage ss;\n \t\tstruct sockaddr *peer = (struct sockaddr *)&ss;\n-- \n1.5.4.3.231.g5d43e\n"}]}