From 3e04c62daab0a8481f907c30414ed246f284a1d9 Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Tue, 18 Oct 2005 18:26:52 -0700 Subject: [PATCH] revised^2: git-daemon extra paranoia, and path DWIM This patch adds some extra paranoia to the git-daemon filename test. In particular, it now rejects pathnames containing //; it also adds a redundant test for pathname absoluteness (belts and suspenders.) A single / at the end of the path is still permitted, however, and the .git and /.git append DWIM stuff is now handled in an integrated manner, which means the resulting path will always be subjected to pathname checks. Signed-off-by: H. Peter Anvin Signed-off-by: Junio C Hamano --- daemon.c | 78 +++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 21 deletions(-) diff --git a/daemon.c b/daemon.c index 11fa3ed11..9ea6c31cd 100644 --- a/daemon.c +++ b/daemon.c @@ -80,17 +80,30 @@ static int path_ok(const char *dir) { const char *p = dir; char **pp; - int sl = 1, ndot = 0; + int sl, ndot; + + /* The pathname here should be an absolute path. */ + if ( *p++ != '/' ) + return 0; + + sl = 1; ndot = 0; for (;;) { if ( *p == '.' ) { ndot++; - } else if ( *p == '/' || *p == '\0' ) { + } else if ( *p == '\0' ) { + /* Reject "." and ".." at the end of the path */ if ( sl && ndot > 0 && ndot < 3 ) - return 0; /* . or .. in path */ + return 0; + + /* Otherwise OK */ + break; + } else if ( *p == '/' ) { + /* Refuse "", "." or ".." */ + if ( sl && ndot < 3 ) + return 0; sl = 1; - if ( *p == '\0' ) - break; /* End of string and all is good */ + ndot = 0; } else { sl = ndot = 0; } @@ -99,7 +112,7 @@ static int path_ok(const char *dir) if ( ok_paths && *ok_paths ) { int ok = 0; - int dirlen = strlen(dir); /* read_packet_line can return embedded \0 */ + int dirlen = strlen(dir); for ( pp = ok_paths ; *pp ; pp++ ) { int len = strlen(*pp); @@ -118,22 +131,16 @@ static int path_ok(const char *dir) return 1; /* Path acceptable */ } -static int upload(char *dir, int dirlen) +static int set_dir(const char *dir) { - loginfo("Request for '%s'", dir); - if (!path_ok(dir)) { - logerror("Forbidden directory: %s\n", dir); + errno = EACCES; return -1; } - if (chdir(dir) < 0) { - logerror("Cannot chdir('%s'): %s", dir, strerror(errno)); + if ( chdir(dir) ) return -1; - } - - chdir(".git"); - + /* * Security on the cheap. * @@ -141,10 +148,39 @@ static int upload(char *dir, int dirlen) * a "git-daemon-export-ok" flag that says that the other side * is ok with us doing this. */ - if ((!export_all_trees && access("git-daemon-export-ok", F_OK)) || - access("objects/", X_OK) || - access("HEAD", R_OK)) { - logerror("Not a valid git-daemon-enabled repository: '%s'", dir); + if (!export_all_trees && access("git-daemon-export-ok", F_OK)) { + errno = EACCES; + return -1; + } + + if (access("objects/", X_OK) || access("HEAD", R_OK)) { + errno = EINVAL; + return -1; + } + + /* If all this passed, we're OK */ + return 0; +} + +static int upload(char *dir) +{ + /* Try paths in this order */ + static const char *paths[] = { "%s", "%s/.git", "%s.git", "%s.git/.git", NULL }; + const char **pp; + /* Enough for the longest path above including final null */ + int buflen = strlen(dir)+10; + char *dirbuf = xmalloc(buflen); + + loginfo("Request for '%s'", dir); + + for ( pp = paths ; *pp ; pp++ ) { + snprintf(dirbuf, buflen, *pp, dir); + if ( !set_dir(dirbuf) ) + break; + } + + if ( !*pp ) { + logerror("Cannot set directory '%s': %s", dir, strerror(errno)); return -1; } @@ -170,7 +206,7 @@ static int execute(void) line[--len] = 0; if (!strncmp("git-upload-pack /", line, 17)) - return upload(line + 16, len - 16); + return upload(line+16); logerror("Protocol error: '%s'", line); return -1; -- 2.26.2