[PATCH] daemon.c and path.enter_repo(): revamp path validation.

The whitelist of git-daemon is checked against return value from
enter_repo(), and enter_repo() used to return the value obtained
from getcwd() to avoid directory aliasing issues as discussed
earier (mid October 2005).

Unfortunately, it did not go well as we hoped.

For example, /pub on a kernel.org public machine is a symlink to
its real mountpoint, and it is understandable that the
administrator does not want to adjust the whitelist every time
/pub needs to point at a different partition for storage
allcation or whatever reasons.  Being able to keep using
/pub/scm as the whitelist is a desirable property.

So this version of enter_repo() reports what it used to chdir()
and validate, but does not use getcwd() to canonicalize the
directory name.  When it sees a user relative path ~user/path,
it internally resolves it to try chdir() there, but it still
reports ~user/path (possibly after appending .git if allowed to
do so, in which case it would report ~user/path.git).

What this means is that if a whitelist wants to allow a user
relative path, it needs to say "~" (for all users) or list user
home directories like "~alice" "~bob".  And no, you cannot say
/home if the advertised way to access user home directories are
~alice,~bob, etc.  The whole point of this is to avoid
unnecessary aliasing issues.

Anyway, because of this, daemon needs to do a bit more work to
guard itself.  Namely, it needs to make sure that the accessor
does not try to exploit its leading path match rule by inserting
/../ in the middle or hanging /.. at the end.  I resurrected the
belts and suspender paranoia code HPA did for this purpose.

This check cannot be done in the enter_repo() unconditionally,
because there are valid callers of enter_repo() that want to
honor /../; authorized users coming over ssh to run send-pack
and fetch-pack should be allowed to do so.

Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/daemon.c b/daemon.c
index 91b96569cd75c77cd4ee169c55be045daccf3908..539f6e87af9824ea331326ade71e343698e235fe 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -82,9 +82,63 @@ static void loginfo(const char *err, ...)
        va_end(params);
 }
 
+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 *dir)
 {
-       char *path = enter_repo(dir, strict_paths);
+       char *path;
+
+       if (avoid_alias(dir)) {
+               logerror("'%s': aliased", dir);
+               return NULL;
+       }
+
+       path = enter_repo(dir, strict_paths);
 
        if (!path) {
                logerror("'%s': unable to chdir or not a git archive", dir);
@@ -96,9 +150,11 @@ static char *path_ok(char *dir)
                int pathlen = strlen(path);
 
                /* The validation is done on the paths after enter_repo
-                * canonicalization, so whitelist should be written in
-                * terms of real pathnames (i.e. after ~user is expanded
-                * and symlinks resolved).
+                * appends optional {.git,.git/.git} and friends, but 
+                * it does not use getcwd().  So if your /pub is
+                * a symlink to /mnt/pub, you can whitelist /pub and
+                * do not have to say /mnt/pub.
+                * Do not say /pub/.
                 */
                for ( pp = ok_paths ; *pp ; pp++ ) {
                        int len = strlen(*pp);

diff --git a/path.c b/path.c
index 2c077c0c8f7066c5338050ce890c0846adedf6bd..334b2bd195f56ac6aaf9f9b8a36fe59115cd758c 100644 (file)
--- a/path.c
+++ b/path.c
@@ -131,76 +131,121 @@ int validate_symref(const char *path)
        return -1;
 }
 
-static char *current_dir(void)
+static char *user_path(char *buf, char *path, int sz)
 {
-       return getcwd(pathname, sizeof(pathname));
-}
-
-static int user_chdir(char *path)
-{
-       char *dir = path;
+       struct passwd *pw;
+       char *slash;
+       int len, baselen;
 
-       if(*dir == '~') {               /* user-relative path */
-               struct passwd *pw;
-               char *slash = strchr(dir, '/');
-
-               dir++;
-               /* '~/' and '~' (no slash) means users own home-dir */
-               if(!*dir || *dir == '/')
-                       pw = getpwuid(getuid());
-               else {
-                       if (slash) {
-                               *slash = '\0';
-                               pw = getpwnam(dir);
-                               *slash = '/';
-                       }
-                       else
-                               pw = getpwnam(dir);
+       if (!path || path[0] != '~')
+               return NULL;
+       path++;
+       slash = strchr(path, '/');
+       if (path[0] == '/' || !path[0]) {
+               pw = getpwuid(getuid());
+       }
+       else {
+               if (slash) {
+                       *slash = 0;
+                       pw = getpwnam(path);
+                       *slash = '/';
                }
-
-               /* make sure we got something back that we can chdir() to */
-               if(!pw || chdir(pw->pw_dir) < 0)
-                       return -1;
-
-               if(!slash || !slash[1]) /* no path following username */
-                       return 0;
-
-               dir = slash + 1;
+               else
+                       pw = getpwnam(path);
        }
-
-       /* ~foo/path/to/repo is now path/to/repo and we're in foo's homedir */
-       if(chdir(dir) < 0)
-               return -1;
-
-       return 0;
+       if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
+               return NULL;
+       baselen = strlen(pw->pw_dir);
+       memcpy(buf, pw->pw_dir, baselen);
+       while ((1 < baselen) && (buf[baselen-1] == '/')) {
+               buf[baselen-1] = 0;
+               baselen--;
+       }
+       if (slash && slash[1]) {
+               len = strlen(slash);
+               if (sz <= baselen + len)
+                       return NULL;
+               memcpy(buf + baselen, slash, len + 1);
+       }
+       return buf;
 }
 
+/*
+ * First, one directory to try is determined by the following algorithm.
+ *
+ * (0) If "strict" is given, the path is used as given and no DWIM is
+ *     done. Otherwise:
+ * (1) "~/path" to mean path under the running user's home directory;
+ * (2) "~user/path" to mean path under named user's home directory;
+ * (3) "relative/path" to mean cwd relative directory; or
+ * (4) "/absolute/path" to mean absolute directory.
+ *
+ * Unless "strict" is given, we try access() for existence of "%s.git/.git",
+ * "%s/.git", "%s.git", "%s" in this order.  The first one that exists is
+ * what we try.
+ *
+ * Second, we try chdir() to that.  Upon failure, we return NULL.
+ *
+ * Then, we try if the current directory is a valid git repository.
+ * Upon failure, we return NULL.
+ *
+ * If all goes well, we return the directory we used to chdir() (but
+ * before ~user is expanded), avoiding getcwd() resolving symbolic
+ * links.  User relative paths are also returned as they are given,
+ * except DWIM suffixing.
+ */
 char *enter_repo(char *path, int strict)
 {
-       if(!path)
+       static char used_path[PATH_MAX];
+       static char validated_path[PATH_MAX];
+
+       if (!path)
                return NULL;
 
-       if (strict) {
-               if (chdir(path) < 0)
+       if (!strict) {
+               static const char *suffix[] = {
+                       ".git/.git", "/.git", ".git", "", NULL,
+               };
+               int len = strlen(path);
+               int i;
+               while ((1 < len) && (path[len-1] == '/')) {
+                       path[len-1] = 0;
+                       len--;
+               }
+               if (PATH_MAX <= len)
                        return NULL;
-       }
-       else {
-               if (!*path)
-                       ; /* happy -- no chdir */
-               else if (!user_chdir(path))
-                       ; /* happy -- as given */
-               else if (!user_chdir(mkpath("%s.git", path)))
-                       ; /* happy -- uemacs --> uemacs.git */
-               else
+               if (path[0] == '~') {
+                       if (!user_path(used_path, path, PATH_MAX))
+                               return NULL;
+                       strcpy(validated_path, path);
+                       path = used_path;
+               }
+               else if (PATH_MAX - 10 < len)
+                       return NULL;
+               else {
+                       path = strcpy(used_path, path);
+                       strcpy(validated_path, path);
+               }
+               len = strlen(path);
+               for (i = 0; suffix[i]; i++) {
+                       strcpy(path + len, suffix[i]);
+                       if (!access(path, F_OK)) {
+                               strcat(validated_path, suffix[i]);
+                               break;
+                       }
+               }
+               if (!suffix[i] || chdir(path))
                        return NULL;
-               (void)chdir(".git");
+               path = validated_path;
        }
+       else if (chdir(path))
+               return NULL;
 
-       if(access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
-          validate_symref("HEAD") == 0) {
+       if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
+           validate_symref("HEAD") == 0) {
                putenv("GIT_DIR=.");
                check_repository_format();
-               return current_dir();
+               return path;
        }
 
        return NULL;