[PATCH] weston-launch: Fixed TTY switching

[Daniel Stone]

On Wednesday, April 8, 2015, Bill Spitzak <spitzak at gmail.com> wrote:

> On 04/07/2015 05:03 PM, Bryce Harrington wrote:
>
>> I'm sure this is not going to ever be a problem since tty filenames and
>> paths are on the short side, but since the tty string is an input
>> parameter to this routine, it would be better defensive programming to
>> use strncpy.
>>
>
> strncpy is not a fix and should never be used.
>
> That is because the proper invocation is:
>
>         strncpy(dest, len-1, src); dest[len-1] = 0;
>
> This is almost always done incorrectly, and even when correct it is hard
> to read and it wastes time filling the buffer with 0 when only the first 0
> needs to be written.
>
> The best solution is to use strlcpy.
>
> If politics make that impossible, use snprintf(dest, len, "%s", src) which
> is exactly the same as strlcpy, including the return value! (imagine
> that...)
>

It's not the politics, it's that silently truncating a filename you're
hoping to use will at best pick a non-existent file, and at worst pick a
totally different/unrelated file.

Unless truncation is explicitly acceptable
(indicative/non-authoritative strings in UI), strlcpy and other
silently-truncating copies are actively harmful, and a potential security
risk.

To be honest though, I'd prefer the library entrypoint existed so the
intention was clear, making it easier to audit for and spot this horrendous
anti-pattern.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150408/89cfcca8/attachment.html>