From: Steven Michalske Date: Wed, 09 Jun 2010 20:42:31 GMT Subject: Re: [PATCH] Use strncpy to protect from buffer overruns. Message-ID: <1E40D9E3-5459-4D29-9D6D-A6528FF8407F@gmail.com> In-Reply-To: On Jun 9, 2010, at 12:31 PM, Alex Riesen wrote: > On Wed, Jun 9, 2010 at 20:25, Steven Michalske wrote: >>> On Wed, Jun 9, 2010 at 12:22, Steven Michalske 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.