{"thread":{"id":"2142","subject":"[PATCH] revised^2: git-daemon extra paranoia, and path DWIM","startedAt":"2005-10-19T01:09:19Z","lastAt":"2005-10-19T01:09:19Z","messageCount":1,"participants":["H. Peter Anvin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"10245","messageId":"43559CBF.2040105@zytor.com","threadId":"2142","inReplyTo":null,"subject":"[PATCH] revised^2: git-daemon extra paranoia, and path DWIM","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-19T01:09:19Z","receivedAt":"2005-10-19T01:09:19Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"This patch adds some extra paranoia to the git-daemon filename test.  In \nparticular, it now rejects pathnames containing //; it also adds a \nredundant test for pathname absoluteness (belts and suspenders.)\n\nA single / at the end of the path is still permitted, however, and the \n.git and /.git append DWIM stuff is now handled in an integrated manner, \nwhich means the resulting path will always be subjected to pathname checks.\n\nSigned-off-by: H. Peter Anvin <hpa@zytor.com>\n\n\ndiff --git a/daemon.c b/daemon.c\n--- a/daemon.c\n+++ b/daemon.c\n@@ -80,17 +80,30 @@ static int path_ok(const char *dir)\n {\n \tconst char *p = dir;\n \tchar **pp;\n-\tint sl = 1, ndot = 0;\n+\tint sl, ndot;\n+\n+\t/* The pathname here should be an absolute path. */\n+\tif ( *p++ != '/' )\n+\t\treturn 0;\n+\n+\tsl = 1;  ndot = 0;\n \n \tfor (;;) {\n \t\tif ( *p == '.' ) {\n \t\t\tndot++;\n-\t\t} else if ( *p == '/' || *p == '\\0' ) {\n+\t\t} else if ( *p == '\\0' ) {\n+\t\t\t/* Reject \".\" and \"..\" at the end of the path */\n \t\t\tif ( sl && ndot > 0 && ndot < 3 )\n-\t\t\t\treturn 0; /* . or .. in path */\n+\t\t\t\treturn 0;\n+\n+\t\t\t/* Otherwise OK */\n+\t\t\tbreak;\n+\t\t} else if ( *p == '/' ) {\n+\t\t\t/* Refuse \"\", \".\" or \"..\" */\n+\t\t\tif ( sl && ndot < 3 )\n+\t\t\t\treturn 0;\n \t\t\tsl = 1;\n-\t\t\tif ( *p == '\\0' )\n-\t\t\t\tbreak; /* End of string and all is good */\n+\t\t\tndot = 0;\n \t\t} else {\n \t\t\tsl = ndot = 0;\n \t\t}\n@@ -99,7 +112,7 @@ static int path_ok(const char *dir)\n \n \tif ( ok_paths && *ok_paths ) {\n \t\tint ok = 0;\n-\t\tint dirlen = strlen(dir); /* read_packet_line can return embedded \\0 */\n+\t\tint dirlen = strlen(dir);\n \n \t\tfor ( pp = ok_paths ; *pp ; pp++ ) {\n \t\t\tint len = strlen(*pp);\n@@ -118,22 +131,16 @@ static int path_ok(const char *dir)\n \treturn 1;\t\t/* Path acceptable */\n }\n \n-static int upload(char *dir, int dirlen)\n+static int set_dir(const char *dir)\n {\n-\tloginfo(\"Request for '%s'\", dir);\n-\n \tif (!path_ok(dir)) {\n-\t\tlogerror(\"Forbidden directory: %s\\n\", dir);\n+\t\terrno = EACCES;\n \t\treturn -1;\n \t}\n \n-\tif (chdir(dir) < 0) {\n-\t\tlogerror(\"Cannot chdir('%s'): %s\", dir, strerror(errno));\n+\tif ( chdir(dir) )\n \t\treturn -1;\n-\t}\n-\n-\tchdir(\".git\");\n-\n+\t\n \t/*\n \t * Security on the cheap.\n \t *\n@@ -141,10 +148,39 @@ static int upload(char *dir, int dirlen)\n \t * a \"git-daemon-export-ok\" flag that says that the other side\n \t * is ok with us doing this.\n \t */\n-\tif ((!export_all_trees && access(\"git-daemon-export-ok\", F_OK)) ||\n-\t    access(\"objects/\", X_OK) ||\n-\t    access(\"HEAD\", R_OK)) {\n-\t\tlogerror(\"Not a valid git-daemon-enabled repository: '%s'\", dir);\n+\tif (!export_all_trees && access(\"git-daemon-export-ok\", F_OK)) {\n+\t\terrno = EACCES;\n+\t\treturn -1;\n+\t}\n+\n+\tif (access(\"objects/\", X_OK) || access(\"HEAD\", R_OK)) {\n+\t\terrno = EINVAL;\n+\t\treturn -1;\n+\t}\n+\n+\t/* If all this passed, we're OK */\n+\treturn 0;\n+}\n+\n+static int upload(char *dir)\n+{\n+\t/* Try paths in this order */\n+\tstatic const char *paths[] = { \"%s\", \"%s/.git\", \"%s.git\", \"%s.git/.git\", NULL };\n+\tconst char **pp;\n+\t/* Enough for the longest path above including final null */\n+\tint buflen = strlen(dir)+10;\n+\tchar *dirbuf = xmalloc(buflen);\n+\n+\tloginfo(\"Request for '%s'\", dir);\n+\n+\tfor ( pp = paths ; *pp ; pp++ ) {\n+\t\tsnprintf(dirbuf, buflen, *pp, dir);\n+\t\tif ( !set_dir(dirbuf) )\n+\t\t\tbreak;\n+\t}\n+\n+\tif ( !*pp ) {\n+\t\tlogerror(\"Cannot set directory '%s': %s\", dir, strerror(errno));\n \t\treturn -1;\n \t}\n \n@@ -170,7 +206,7 @@ static int execute(void)\n \t\tline[--len] = 0;\n \n \tif (!strncmp(\"git-upload-pack /\", line, 17))\n-\t\treturn upload(line + 16, len - 16);\n+\t\treturn upload(line+16);\n \n \tlogerror(\"Protocol error: '%s'\", line);\n \treturn -1;\n"}]}