Re: [PATCH] Use strncpy to protect from buffer overruns.
On Jun 9, 2010, at 12:31 PM, Alex Riesen wrote:
Show 18 quoted lines
> On Wed, Jun 9, 2010 at 20:25, Steven Michalske <smichalske@gmail.com> wrote:
>>> On Wed, Jun 9, 2010 at 12:22, Steven Michalske <smichalske@gmail.com> 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.
Show 14 quoted lines
>>>> - 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.
Steven Michalske <smichalske@gmail.com>