On 08/13/2014 04:31 PM, Kevin Grittner wrote:
> David Rowley <dgrowleyml@gmail.com> wrote:
>
>> I had a quick look at the usages of strncpy in master tonight and
>> I've really just picked out the obviously broken ones for now.
>> The other ones, on first look, either look safe, or require some
>> more analysis to see what's actually done with the string.
>>
>> Does anyone disagree with the 2 changes in the attached?
>
> I am concerned that failure to check for truncation could allow
> deletion of unexpected files or directories. While this is
> probably not as dangerous as *executing* unexpected files, it seems
> potentially problematic. At the very least, a code comment
> explaining why calling unlink on something which is not what
> appears to be expected is not a problem there.
>
> Some might consider it overkill, but I tend to draw a pretty hard
> line on deleting or executing random files, even if the odds seem
> to be that the mangled name won't find a match. Granted, those
> problems exist now, but without checking for truncation it seems to
> me that we're just deleting *different* incorrect filenames, not
> really fixing the problem.
strlcpy is clearly better than strncpy here, but I wonder if we should
have yet another string copying function that throws an error instead of
truncating, if the buffer is too small. What you really want in these
cases is a "path too long" error.
- Heikki