Re: strscpy

Maxime Villard <[email protected]> Sat, 11 Jul 2020 09:25:34 +0200
Newsgroups gmane.os.netbsd.devel.security
Message-ID <[email protected]>
Le 10/06/2020 à 18:33, Maxime Villard a écrit :
> Le 05/06/2020 à 19:50, Maxime Villard a écrit :
>> Le 04/06/2020 à 01:02, Taylor R Campbell a écrit :
>>>> Date: Tue, 2 Jun 2020 20:25:45 +0200
>>>> From: Maxime Villard <[email protected]>
>>>>
>>>> Here is a patch that converts all of the copystr() calls in the kernel to
>>>> strkcpy(). One exception is vfs_subr.c where it will be done differently
>>>> and isn't included here.
>>>>
>>>>     https://m00nbsd.net/garbage/libkern/strkcpy.diff
>>
>> [...]
>>
>> Considering that the error checking of strkcpy (or whatever we call it)
>> is the same as strlcpy's, you can expect to be able to do 'l'->'k' in a
>> mechanical manner in the vast majority of cases.
>>
>> As an example, take sys/kern/. The majority of calls to strlcpy do not
>> check the return value. So you can do 'l'->'k' automatically. Four files
>> have calls that check the return value: sys_module.c, kern_exec.c,
>> uipc_domain.c, kern_sysctl.c. In each of these cases, you can do 'l'->'k'
>> automatically too, because the error checking is the same.
> 
> So what do people think about this?

Note: actually, I was wrong when I said that copystr offers the same level
of safety as strkcpy. It does not, because copystr does not guarantee NUL
termination if the string is too long, as opposed to strkcpy.

That means in practice that the return value of copystr should always be
checked, and the final '\0' should always be added manually (if we accept
the truncation and proceed regardless).

Therefore strkcpy is better than copystr, in terms of "safest result with
least effort".

I will go with the last iteration of strkcpy we ended up with, that is:

	https://m00nbsd.net/garbage/libkern/strkcpy.diff

Around ~five copystr will remain as copystr, and the rest of the copystr
and strlcpy (and strcpy) shall be switched to strkcpy.

Maxime