Re: strscpy

Maxime Villard <[email protected]> Sat, 30 May 2020 08:52:39 +0200
Newsgroups gmane.os.netbsd.devel.security
Message-ID <[email protected]>
Le 29/05/2020 à 21:34, Taylor R Campbell a écrit :
>> Date: Fri, 29 May 2020 20:22:34 +0200
>> From: Maxime Villard <[email protected]>
>>
>> I have updated my patch:
>>
>> 	https://m00nbsd.net/garbage/libkern/strscpy.diff
>>
>> We simply return -1. Will commit soon unless there is valid objection.
> 
> We shouldn't use the same name if we're implementing different
> semantics.
> 
> Do we actually need it to return the length?

Not returning it would be widening the difference in semantics, but
see below:

> There are over 5000 strlcpy calls in tree.  At most 250 of them (many
> of these false positives) use the return value at all.
> 
> I searched through the cases that might use the return value to find
> how any of them use it.  Every case I found, except for the definition
> of strlcat and a libevent regression test for strlcpy itself, uses the
> return value at most to detect truncation and for no other purpose.
> (sockaddr_format returns whatever strlcpy returns, but nothing uses
> its return value.)
> 
> Let's just make it return zero for success and -1 for failure --
> failure meaning: it was truncated before a NUL byte in the source.

Correct, but please keep in mind that the goal is also to replace
copystr(), and there are a few (<10) cases where the caller wants
the len.

If strscpy() doesn't provide the length, the callers would have to
call strlen(), and it seems like asking more effort than should be
necessary. Unless it is fine? Or unless we decide to keep copystr()
for a very small number of selected places?

(Side note: I just noticed that FreeBSD is dropping copystr(), and
replacing it by... strlcpy(). Which is a bad move; they don't seem
to have realized that strlcpy() is less safe than copystr(). I
guess I should tell them.)

Maxime