Re: [PATCHv2 1/2] cld: fix CLD_INODE_NAME_MAX woes

On Wed, Feb 3, 2010 at 2:27 PM, Jeff Garzik <[email protected]> wrote:
> On 02/03/2010 08:45 AM, Colin McCabe wrote:
>>
>> When we create a static buffer for an inode name, and treat it like a
>> null-terminated string, it needs to be of length CLD_INODE_NAME_MAX + 1 so
>> that it can hold the NULL-terminator.
>>
>> In cldc_del and cldc_open, we should check that the user-submitted inode
>> name
>> is less than or equal to CLD_INODE_NAME_MAX. Formerly we were just
>> checking
>> that it wasn't too big to fit in the packet.
>>
>> When copying the inode name out of struct cld_dirent_cur, use snprintf
>> rather
>> than strcpy to ensure that we never overflow the buffer. This isn't
>> strictly
>> necessary if all other checks are working perfectly, but it seems prudent.
>>
>> Signed-off-by: Colin McCabe<[email protected]>
>
> applied, after s/snprintf/strncpy/
>
> In general, too, you should never pass a variable string into snprintf, as
> that may make a program vulnerable to printf format string attacks (user
> supplies "%s" as a username, for example).

Oh, how embarassing. I hate it when people do that, now I did it too.

I tend to avoid strncpy because of how it always writes n bytes. If
only glibc had picked up strlcpy.

Colin