From d79374c7b58d3814ffdc277de608243f8e665e3a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 3 Dec 2005 01:45:57 -0800 Subject: [PATCH] [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 --- daemon.c | 64 +++++++++++++++++++++-- path.c | 153 +++++++++++++++++++++++++++++++++++-------------------- 2 files changed, 159 insertions(+), 58 deletions(-) diff --git a/daemon.c b/daemon.c index 91b96569c..539f6e87a 100644 --- 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 2c077c0c8..334b2bd19 100644 --- 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; -- 2.26.2