tech-security archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: strscpy



    Date:        Wed, 3 Jun 2020 23:02:20 +0000
    From:        Taylor R Campbell <campbell+netbsd-tech-security%mumble.net@localhost>
    Message-ID:  <20200603230244.3248560AAD%jupiter.mumble.net@localhost>

  | - returning either len or len+1 is confusing because the meaning is
  |   _sometimes_ strlen of the output but _sometimes_ the size you need
  |   to memcpy to get the same C string as the output

Ignoring len+1 which I think now was the wrong suggestion, I don't
follow this at all.

In similar cases, it would be acceptable to return 0 to indicate
failure, and a length to indicate success (that's more or less what read()
does) - that doesn't work here, as if the src is "", then the length
copied is 0 (no error, indicates success).

If the definition of the function were changed to return the length written,
including the \0 appended, then 0 would be an impossible return value,
and could be used to indicate failure.

If that were done, I doubt anyone would object, it's just "obvious" that
if a value is returned that cannot be the length copied, then it isn't,
and instead is just a failure indicator.

But back to returning the strlen() of the result on success, 0 is no
longer available as invalid (it isn't) - but the value we know cannot ever
be returned is "len".   That is, that is the buffer size, and into that
buffer we must fit the string copied, plus a \0 byte.   The \0 takes one
of the len bytes, so the max length of the copied string is len-1 which is
therefore the biggest possible valid return value.  Returning len indicates
failure, it doesn't indicate "the size you need to memcpy to get the same C
string", which it wouldn't in any case, consider:

	strkcpy(dst, "abcd", 2);

In the "return len" case, that would return 2, as the src string doesn't fit
in the buffer.   But

	memcpy(dst, "abcd", 2);

while it (probably) affects the dst buffer much the same way the
failed strkcpy() call did, does not produce a "C string" as its result.

kre

ps: all that said, I am no fan of replacing the unbroken copystr() calls
by anything at all, leave all those alone, just replace strlcpy().



Home | Main Index | Thread Index | Old Index