On Sat, 27 Jan 2018, Dimitry Andric wrote:

On 27 Jan 2018, at 23:20, Ed Schouten <e...@nuxi.nl> wrote:

2018-01-27 23:16 GMT+01:00 Pedro F. Giffuni <p...@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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to