On Jun 9, 2010, at 12:31 PM, Alex Riesen wrote: > On Wed, Jun 9, 2010 at 20:25, Steven Michalske <smichalske@xxxxxxxxx> wrote: >>> On Wed, Jun 9, 2010 at 12:22, Steven Michalske <smichalske@xxxxxxxxx> wrote: >>>> is_git_directory() uses strcpy with pointer arithmitic, protect it from >>>> overflowing. Even though we currently protect higher up when we have the >>>> environment variable path passed in, we should protect the calls here. >>> >>> Why? The function is static. >>> >> The code might be locally constrained. >> >> I always assume that a bit of code can be overwritten from other portions of code. >> >> A small vulnerability is discovered that lets an attacker remove the length check >> or edit the pointer in the function call, but could not squeeze in the full shell code >> snippet. But the now edited function here lets you put in arbitrarily long code. > > Eh? > Basically the protection is not robust against malicious code. It's armored with leather, not the modern full body armor. >>>> - strcpy(path, suspect); >>>> + path[sizeof(path) - 1] = '\0'; >>>> + >>>> + strncpy(path, suspect, sizeof(path) - 1); >>> >>> And we have strlcpy for such things. >> >> It is not portable. > > Git has its own copy of the function: > > $ git ls-files *strlcpy.c > > $ Good to know, I could refactor with this.-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html