{"thread":{"id":"21546","subject":"[PATCH] http-backend: Protect GIT_PROJECT_ROOT from /../ requests","startedAt":"2009-11-09T19:26:43Z","lastAt":"2009-11-09T19:26:43Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127187","messageId":"20091109192643.GA29299@spearce.org","threadId":"21546","inReplyTo":null,"subject":"[PATCH] http-backend: Protect GIT_PROJECT_ROOT from /../ requests","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-11-09T19:26:43Z","receivedAt":"2009-11-09T19:26:43Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eons ago HPA taught git-daemon how to protect itself from /../\nattacks, which Junio brought back into service in d79374c7b58d\n(\"daemon.c and path.enter_repo(): revamp path validation\").\n\nI did not carry this into git-http-backend as originally we relied\nonly upon PATH_TRANSLATED, and assumed the HTTP server had done\nits access control checks to validate the resolved path was within\na directory permitting access from the remote client.  This would\nusually be sufficient to protect a server from requests for its\n/etc/passwd file by http://host/smart/../etc/passwd sorts of URLs.\n\nHowever in 917adc036086 Mark Lodato added GIT_PROJECT_ROOT as an\nadditional method of configuring the CGI.  When this environment\nvariable is used the web server does not generate the final access\npath and therefore may blindly pass through \"/../etc/passwd\"\nin PATH_INFO under the assumption that \"/../\" might have special\nmeaning to the invoked CGI.\n\nInstead of permitting these sorts of malformed path requests, we\nnow reject them back at the client, with an error message for the\nserver log.  This matches git-daemon behavior.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n cache.h                 |    1 +\n daemon.c                |   49 +----------------------------------------------\n http-backend.c          |    6 +++++\n path.c                  |   47 +++++++++++++++++++++++++++++++++++++++++++++\n t/t5560-http-backend.sh |   31 +++++++++++++++++++++++++++++\n 5 files changed, 86 insertions(+), 48 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4e283be..ecbd88a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -656,6 +656,7 @@ const char *make_relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n char *strip_path_suffix(const char *path, const char *suffix);\n+int daemon_avoid_alias(const char *path);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\ndiff --git a/daemon.c b/daemon.c\nindex 1b5ada6..ce48006 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -101,53 +101,6 @@ static void NORETURN daemon_die(const char *err, va_list params)\n \texit(1);\n }\n \n-static int avoid_alias(char *p)\n-{\n-\tint sl, ndot;\n-\n-\t/*\n-\t * This resurrects the belts and suspenders paranoia check by HPA\n-\t * done in <435560F7.4080006@zytor.com> thread, now enter_repo()\n-\t * does not do getcwd() based path canonicalizations.\n-\t *\n-\t * sl becomes true immediately after seeing '/' and continues to\n-\t * be true as long as dots continue after that without intervening\n-\t * non-dot character.\n-\t */\n-\tif (!p || (*p != '/' && *p != '~'))\n-\t\treturn -1;\n-\tsl = 1; ndot = 0;\n-\tp++;\n-\n-\twhile (1) {\n-\t\tchar ch = *p++;\n-\t\tif (sl) {\n-\t\t\tif (ch == '.')\n-\t\t\t\tndot++;\n-\t\t\telse if (ch == '/') {\n-\t\t\t\tif (ndot < 3)\n-\t\t\t\t\t/* reject //, /./ and /../ */\n-\t\t\t\t\treturn -1;\n-\t\t\t\tndot = 0;\n-\t\t\t}\n-\t\t\telse if (ch == 0) {\n-\t\t\t\tif (0 < ndot && ndot < 3)\n-\t\t\t\t\t/* reject /.$ and /..$ */\n-\t\t\t\t\treturn -1;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t\telse\n-\t\t\t\tsl = ndot = 0;\n-\t\t}\n-\t\telse if (ch == 0)\n-\t\t\treturn 0;\n-\t\telse if (ch == '/') {\n-\t\t\tsl = 1;\n-\t\t\tndot = 0;\n-\t\t}\n-\t}\n-}\n-\n static char *path_ok(char *directory)\n {\n \tstatic char rpath[PATH_MAX];\n@@ -157,7 +110,7 @@ static char *path_ok(char *directory)\n \n \tdir = directory;\n \n-\tif (avoid_alias(dir)) {\n+\tif (daemon_avoid_alias(dir)) {\n \t\tlogerror(\"'%s': aliased\", dir);\n \t\treturn NULL;\n \t}\ndiff --git a/http-backend.c b/http-backend.c\nindex 646e910..f8ea9d7 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -559,7 +559,13 @@ static char* getdir(void)\n \tif (root && *root) {\n \t\tif (!pathinfo || !*pathinfo)\n \t\t\tdie(\"GIT_PROJECT_ROOT is set but PATH_INFO is not\");\n+\t\tif (daemon_avoid_alias(pathinfo))\n+\t\t\tdie(\"'%s': aliased\", pathinfo);\n \t\tstrbuf_addstr(&buf, root);\n+\t\tif (buf.buf[buf.len - 1] != '/')\n+\t\t\tstrbuf_addch(&buf, '/');\n+\t\tif (pathinfo[0] == '/')\n+\t\t\tpathinfo++;\n \t\tstrbuf_addstr(&buf, pathinfo);\n \t\treturn strbuf_detach(&buf, NULL);\n \t} else if (path && *path) {\ndiff --git a/path.c b/path.c\nindex 047fdb0..c7679be 100644\n--- a/path.c\n+++ b/path.c\n@@ -564,3 +564,50 @@ char *strip_path_suffix(const char *path, const char *suffix)\n \t\treturn NULL;\n \treturn xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n }\n+\n+int daemon_avoid_alias(const char *p)\n+{\n+\tint sl, ndot;\n+\n+\t/*\n+\t * This resurrects the belts and suspenders paranoia check by HPA\n+\t * done in <435560F7.4080006@zytor.com> thread, now enter_repo()\n+\t * does not do getcwd() based path canonicalizations.\n+\t *\n+\t * sl becomes true immediately after seeing '/' and continues to\n+\t * be true as long as dots continue after that without intervening\n+\t * non-dot character.\n+\t */\n+\tif (!p || (*p != '/' && *p != '~'))\n+\t\treturn -1;\n+\tsl = 1; ndot = 0;\n+\tp++;\n+\n+\twhile (1) {\n+\t\tchar ch = *p++;\n+\t\tif (sl) {\n+\t\t\tif (ch == '.')\n+\t\t\t\tndot++;\n+\t\t\telse if (ch == '/') {\n+\t\t\t\tif (ndot < 3)\n+\t\t\t\t\t/* reject //, /./ and /../ */\n+\t\t\t\t\treturn -1;\n+\t\t\t\tndot = 0;\n+\t\t\t}\n+\t\t\telse if (ch == 0) {\n+\t\t\t\tif (0 < ndot && ndot < 3)\n+\t\t\t\t\t/* reject /.$ and /..$ */\n+\t\t\t\t\treturn -1;\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tsl = ndot = 0;\n+\t\t}\n+\t\telse if (ch == 0)\n+\t\t\treturn 0;\n+\t\telse if (ch == '/') {\n+\t\t\tsl = 1;\n+\t\t\tndot = 0;\n+\t\t}\n+\t}\n+}\ndiff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh\nindex 908ba07..ed034bc 100755\n--- a/t/t5560-http-backend.sh\n+++ b/t/t5560-http-backend.sh\n@@ -146,6 +146,37 @@ test_expect_success 'http.receivepack false' '\n \tPOST git-receive-pack 0000 \"403 Forbidden\"\n '\n \n+run_backend() {\n+\tREQUEST_METHOD=GET \\\n+\tGIT_PROJECT_ROOT=\"$HTTPD_DOCUMENT_ROOT_PATH\" \\\n+\tPATH_INFO=\"$2\" \\\n+\tgit http-backend >act.out 2>act.err\n+}\n+\n+path_info() {\n+\tif test $1 = 0; then\n+\t\trun_backend \"$2\"\n+\telse\n+\t\ttest_must_fail run_backend \"$2\" &&\n+\t\techo \"fatal: '$2': aliased\" >exp.err &&\n+\t\ttest_cmp exp.err act.err\n+\tfi\n+}\n+\n+test_expect_success 'http-backend blocks bad PATH_INFO' '\n+\tconfig http.getanyfile true &&\n+\n+\trun_backend 0 /repo.git/HEAD &&\n+\n+\trun_backend 1 /repo.git/../HEAD &&\n+\trun_backend 1 /../etc/passwd &&\n+\trun_backend 1 ../etc/passwd &&\n+\trun_backend 1 /etc//passwd &&\n+\trun_backend 1 /etc/./passwd &&\n+\trun_backend 1 /etc/.../passwd &&\n+\trun_backend 1 //domain/data.txt\n+'\n+\n cat >exp <<EOF\n \n ###  refs/heads/master\n-- \n1.6.5.2.351.g09432\n"}]}