From 9cb18f56fdee6885884f5f08bd5335a42d9034dc Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 3 Jul 2007 12:40:20 +0200 Subject: [PATCH] Don't smash stack when $GIT_ALTERNATE_OBJECT_DIRECTORIES is too long There is no restriction on the length of the name returned by get_object_directory, other than the fact that it must be a stat'able git object directory. That means its name may have length up to PATH_MAX-1 (i.e., often 4095) not counting the trailing NUL. Combine that with the assumption that the concatenation of that name and suffixes like "/info/alternates" and "/pack/---long-name---.idx" will fit in a buffer of length PATH_MAX, and you see the problem. Here's a fix: sha1_file.c (prepare_packed_git_one): Lengthen "path" buffer so we are guaranteed to be able to append "/pack/" without checking. Skip any directory entry that is too long to be appended. (read_info_alternates): Protect against a similar buffer overrun. Before this change, using the following admittedly contrived environment setting would cause many git commands to clobber their stack and segfault on a system with PATH_MAX == 4096: t=$(perl -e '$s=".git/objects";$n=(4096-6-length($s))/2;print "./"x$n . $s') export GIT_ALTERNATE_OBJECT_DIRECTORIES=$t touch g ./git-update-index --add g If you run the above commands, you'll soon notice that many git commands now segfault, so you'll want to do this: unset GIT_ALTERNATE_OBJECT_DIRECTORIES Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- sha1_file.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index b9d07de15..1d328c8d6 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -352,10 +352,14 @@ static void read_info_alternates(const char * relative_base, int depth) char *map; size_t mapsz; struct stat st; - char path[PATH_MAX]; + const char alt_file_name[] = "info/alternates"; + /* Given that relative_base is no longer than PATH_MAX, + ensure that "path" has enough space to append "/", the + file name, "info/alternates", and a trailing NUL. */ + char path[PATH_MAX + 1 + sizeof alt_file_name]; int fd; - sprintf(path, "%s/info/alternates", relative_base); + sprintf(path, "%s/%s", relative_base, alt_file_name); fd = open(path, O_RDONLY); if (fd < 0) return; @@ -814,7 +818,10 @@ void install_packed_git(struct packed_git *pack) static void prepare_packed_git_one(char *objdir, int local) { - char path[PATH_MAX]; + /* Ensure that this buffer is large enough so that we can + append "/pack/" without clobbering the stack even if + strlen(objdir) were PATH_MAX. */ + char path[PATH_MAX + 1 + 4 + 1 + 1]; int len; DIR *dir; struct dirent *de; @@ -836,6 +843,9 @@ static void prepare_packed_git_one(char *objdir, int local) if (!has_extension(de->d_name, ".idx")) continue; + if (len + namelen + 1 > sizeof(path)) + continue; + /* Don't reopen a pack we already have. */ strcpy(path + len, de->d_name); for (p = packed_git; p; p = p->next) { -- 2.26.2