http-backend: Protect GIT_PROJECT_ROOT from /../ requests

Eons ago HPA taught git-daemon how to protect itself from /../
attacks, which Junio brought back into service in d79374c7b58d
("daemon.c and path.enter_repo(): revamp path validation").

I did not carry this into git-http-backend as originally we relied
only upon PATH_TRANSLATED, and assumed the HTTP server had done
its access control checks to validate the resolved path was within
a directory permitting access from the remote client.  This would
usually be sufficient to protect a server from requests for its
/etc/passwd file by http://host/smart/../etc/passwd sorts of URLs.

However in 917adc036086 Mark Lodato added GIT_PROJECT_ROOT as an
additional method of configuring the CGI.  When this environment
variable is used the web server does not generate the final access
path and therefore may blindly pass through "/../etc/passwd"
in PATH_INFO under the assumption that "/../" might have special
meaning to the invoked CGI.

Instead of permitting these sorts of malformed path requests, we
now reject them back at the client, with an error message for the
server log.  This matches git-daemon behavior.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index 4e283be24e1286ea091534b578653df7fe73b1e4..ecbd88adba612443d98e505eb1e7303e5fc7c3b0 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -656,6 +656,7 @@ const char *make_relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
 int longest_ancestor_length(const char *path, const char *prefix_list);
 char *strip_path_suffix(const char *path, const char *suffix);
+int daemon_avoid_alias(const char *path);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);

diff --git a/daemon.c b/daemon.c
index 1b5ada6648da66d14dec6c399898916920582852..ce4800621cece33de68991b52419312803875f84 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -101,53 +101,6 @@ static void NORETURN daemon_die(const char *err, va_list params)
        exit(1);
 }
 
-static int avoid_alias(char *p)
-{
-       int sl, ndot;
-
-       /*
-        * This resurrects the belts and suspenders paranoia check by HPA
-        * done in <435560F7.4080006@zytor.com> thread, now enter_repo()
-        * does not do getcwd() based path canonicalizations.
-        *
-        * sl becomes true immediately after seeing '/' and continues to
-        * be true as long as dots continue after that without intervening
-        * non-dot character.
-        */
-       if (!p || (*p != '/' && *p != '~'))
-               return -1;
-       sl = 1; ndot = 0;
-       p++;
-
-       while (1) {
-               char ch = *p++;
-               if (sl) {
-                       if (ch == '.')
-                               ndot++;
-                       else if (ch == '/') {
-                               if (ndot < 3)
-                                       /* reject //, /./ and /../ */
-                                       return -1;
-                               ndot = 0;
-                       }
-                       else if (ch == 0) {
-                               if (0 < ndot && ndot < 3)
-                                       /* reject /.$ and /..$ */
-                                       return -1;
-                               return 0;
-                       }
-                       else
-                               sl = ndot = 0;
-               }
-               else if (ch == 0)
-                       return 0;
-               else if (ch == '/') {
-                       sl = 1;
-                       ndot = 0;
-               }
-       }
-}
-
 static char *path_ok(char *directory)
 {
        static char rpath[PATH_MAX];
@@ -157,7 +110,7 @@ static char *path_ok(char *directory)
 
        dir = directory;
 
-       if (avoid_alias(dir)) {
+       if (daemon_avoid_alias(dir)) {
                logerror("'%s': aliased", dir);
                return NULL;
        }

diff --git a/http-backend.c b/http-backend.c
index 646e9108bddbd538f04fc01129214aa8615d5e5a..f8ea9d7faa0494375d3d5413e91869d74252d649 100644 (file)
--- a/http-backend.c
+++ b/http-backend.c
@@ -559,7 +559,13 @@ static char* getdir(void)
        if (root && *root) {
                if (!pathinfo || !*pathinfo)
                        die("GIT_PROJECT_ROOT is set but PATH_INFO is not");
+               if (daemon_avoid_alias(pathinfo))
+                       die("'%s': aliased", pathinfo);
                strbuf_addstr(&buf, root);
+               if (buf.buf[buf.len - 1] != '/')
+                       strbuf_addch(&buf, '/');
+               if (pathinfo[0] == '/')
+                       pathinfo++;
                strbuf_addstr(&buf, pathinfo);
                return strbuf_detach(&buf, NULL);
        } else if (path && *path) {

diff --git a/path.c b/path.c
index 047fdb0a1fe8151f5f275ca5333365df786a8abd..c7679be5c8727c7d374338f3ed047d8a5ec25c3c 100644 (file)
--- a/path.c
+++ b/path.c
@@ -564,3 +564,50 @@ char *strip_path_suffix(const char *path, const char *suffix)
                return NULL;
        return xstrndup(path, chomp_trailing_dir_sep(path, path_len));
 }
+
+int daemon_avoid_alias(const char *p)
+{
+       int sl, ndot;
+
+       /*
+        * This resurrects the belts and suspenders paranoia check by HPA
+        * done in <435560F7.4080006@zytor.com> thread, now enter_repo()
+        * does not do getcwd() based path canonicalizations.
+        *
+        * sl becomes true immediately after seeing '/' and continues to
+        * be true as long as dots continue after that without intervening
+        * non-dot character.
+        */
+       if (!p || (*p != '/' && *p != '~'))
+               return -1;
+       sl = 1; ndot = 0;
+       p++;
+
+       while (1) {
+               char ch = *p++;
+               if (sl) {
+                       if (ch == '.')
+                               ndot++;
+                       else if (ch == '/') {
+                               if (ndot < 3)
+                                       /* reject //, /./ and /../ */
+                                       return -1;
+                               ndot = 0;
+                       }
+                       else if (ch == 0) {
+                               if (0 < ndot && ndot < 3)
+                                       /* reject /.$ and /..$ */
+                                       return -1;
+                               return 0;
+                       }
+                       else
+                               sl = ndot = 0;
+               }
+               else if (ch == 0)
+                       return 0;
+               else if (ch == '/') {
+                       sl = 1;
+                       ndot = 0;
+               }
+       }
+}

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 908ba079d27b8273521f62f305b2577c670869a4..ed034bc980ca6636db4acda443d15426e470c25b 100755 (executable)
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -146,6 +146,37 @@ test_expect_success 'http.receivepack false' '
        POST git-receive-pack 0000 "403 Forbidden"
 '
 
+run_backend() {
+       REQUEST_METHOD=GET \
+       GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+       PATH_INFO="$2" \
+       git http-backend >act.out 2>act.err
+}
+
+path_info() {
+       if test $1 = 0; then
+               run_backend "$2"
+       else
+               test_must_fail run_backend "$2" &&
+               echo "fatal: '$2': aliased" >exp.err &&
+               test_cmp exp.err act.err
+       fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+       config http.getanyfile true &&
+
+       run_backend 0 /repo.git/HEAD &&
+
+       run_backend 1 /repo.git/../HEAD &&
+       run_backend 1 /../etc/passwd &&
+       run_backend 1 ../etc/passwd &&
+       run_backend 1 /etc//passwd &&
+       run_backend 1 /etc/./passwd &&
+       run_backend 1 /etc/.../passwd &&
+       run_backend 1 //domain/data.txt
+'
+
 cat >exp <<EOF
 
 ###  refs/heads/master