Re: strlcpy and strlcat support

[Paul Eggert <eggert at cs dot ucla dot edu>]

On 09/13/11 10:25, Dmitrieva Liubov wrote:
> I just want to know up-to-date vision of this issue.

Hmm, would it help to have an FAQ for this?

Many reasons to avoid strlcpy and strlcat are just as valid now as
they were ten years ago.  When this issue came up back then, I looked
at OpenSSH (which had been heavily modified to use strlcat and
strlcpy), and found that all uses of strlcat and strlcpy that I
checked were either (1) not necessary, as strcat/strcpy could not
overflow the buffer, or (2) harmful, in that they silently truncated
strings.

To see whether things have changed in the past decade, I just now
looked at the source code of OpenSSH 5.9p1 (the latest release), and
looked for calls to these functions, using the command "egrep -n
'strlcpy|strlcat' `find * -type f -print|sort`".  Of the first five
calls I examined, the first and fifth were buggy (in that data could
be silently truncated), and in the other three the use of
strlcpy/strlcat was unnecessary (strcpy and strcat would have
worked just as well) .  I stopped looking after five calls because I
didn't want to waste my time further.

If even flagship security software like OpenSSH has issues like this
when using strlcpy and strlcat, it's a pretty good sign that the
primitives have problems.  We shouldn't waste our time encouraging
others to use error-prone primitives, and we certainly shouldn't waste
our time implementing optimized versions of error-prone primitives.

For the record, here are the five calls I mentioned.  I'll leave it as
an exercise for the reader to find the issues; typically you'll need
to read the surrounding code.

addrmatch.c:321:	if (p == NULL || strlcpy(addrbuf, p, sizeof(addrbuf)) > sizeof(addrbuf))
auth-pam.c:742:			strlcpy(**prompts + plen, msg, len - plen);
auth-pam.c:752:			strlcpy(**prompts + plen, msg, len - plen);
auth-pam.c:754:			strlcat(**prompts + plen, "\n", len - plen);
auth-rhosts.c:110:			strlcpy(userbuf, server_user, sizeof(userbuf));