From d79374c7b58d3814ffdc277de608243f8e665e3a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 3 Dec 2005 01:45:57 -0800 Subject: [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 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 4 deletions(-) (limited to 'daemon.c') diff --git a/daemon.c b/daemon.c index 91b96569cd..539f6e87af 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); -- cgit v1.2.3