svn commit: r328492 - head/contrib/opie/libopie

[Bruce Evans]

On Sat, 27 Jan 2018, Dimitry Andric wrote:

> On 27 Jan 2018, at 23:20, Ed Schouten <ed at nuxi.nl> wrote:
>>
>> 2018-01-27 23:16 GMT+01:00 Pedro F. Giffuni <pfg at freebsd.org>:
>>>        char host[sizeof(utmp.ut_host) + 1];
>>>        insecure = 1;
>>>
>>> -       strncpy(host, utmp.ut_host, sizeof(utmp.ut_host));
>>> -       host[sizeof(utmp.ut_host)] = 0;
>>> +       strncpy(host, utmp.ut_host, sizeof(host));
>>
>> Wait... This may access utmp.ut_host one byte past the end and no
>> longer guarantees that host is null-terminated, right?

> No, strncpy "copies at most len characters from src into dst".  However,

No, the change breaks the length so 1 byte past the end is accessed
in implementations where ut_host is not guaranteed to be NUL terminated
and the current instance of ut_host is not NUL terminated.

> if the length of the source is equal to or greater than len, the
> destination is *not* null terminated.  This is likely why the
> "host[sizeof(utmp.ut_host)] = 0;" statement was added.

This is why that statement was there.

This change is not even wrong under FreeBSD, since ut_host and several other
fields are guaranteed to be NUL terminated in the FreeBSD implementation.
The code was correct and portable and the change just breaks its portability.

> In any case, this is why strlcpy exists. :)

Using strlcpy() in libopie would be another good unportabilization.
contrib/opie never uses strlc*() except in 1 place previously
unportabilized in r208586.  That at least fixed 2 bugs (2 related off
by 1 errors in the code intended to avoid buffer overruns, with the
result that buffer overruns were limited to 1 byte).  It moved the
style bugs by changing hacking on the source string to use of strlcpy().

Bruce