From: Steven Michalske <smichalske@gmail.com>
Date: Wed, 09 Jun 2010 18:25:44 GMT
Subject: Re: [PATCH] Use strncpy to protect from buffer overruns.
Message-ID: <34152ED6-ACCC-467B-9076-1A742612AC75@gmail.com>
In-Reply-To: <AANLkTikID-cEqdDD8XEEYmClSYEkcxdAU-rjYA2LoRVe@mail.gmail.com>


On Jun 9, 2010, at 5:44 AM, Alex Riesen 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.

>> -       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.