tech-security archive

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

Re: strscpy



    Date:        Sat, 30 May 2020 15:41:39 +0000
    From:        Taylor R Campbell <campbell+netbsd-tech-security%mumble.net@localhost>
    Message-ID:  <20200530154159.0B34860AFB%jupiter.mumble.net@localhost>

  | Of course, if strwhatevercpy is _guaranteed_ to return the length of
  | the output string,

That wasn't the intent of my suggestion, it would still be "length or error"
just the error would be a length value, rather than -1.

So, unless it is guaranteed that sizeof(dst) > strlen(src1)
(perhaps because dst isn't an array, but is malloc'd to be big enough)
this would be the correct idiom:

  | 	n = strlcpy(dst, src1, sizeof(dst));
  | 	if (n >= sizeof(dst))
  | 		goto fail;
  | 	strlcpy(dst + n, src2, sizeof(dst) - n);

using whatever name is finally decided upon.

  | - Returning size (or size+1) might be safer than strlcpy.  But it has
  |   the (to my mind) counterintuitive property that the result is
  |   sometimes the length _without_ the NUL terminator and the result is
  |   sometimes the size _with_ the NUL terminator (or with the NUL
  |   terminator and one more, for size+1).

You don't want to think of the size+1 (or SIZE_MAX or whatever) as being
the length of anything - it is rather an indicator that truncation happened.

  | - Using SIZE_MAX avoids the casts or ssize_t needed by -1, and means
  |   if you try to use the length in the truncation case, you will almost
  |   certainly cause a fault that will abort the application or panic the
  |   kernel -- in contrast to a one- or two-byte buffer overrun of size
  |   or size+1 which is much less likely to be noticed.

Except that SIZE_MAX *is* -1 (or (unsigned)-1) so when added to something
it effectively just decrements by 1.

That said, either way would work (size+1 or SIZE_MAX ... personally I
prefer not just size - even though that should never be returned for a
valid copy) ... but upon reflection, size+1 might not be the best choice,
as it could overflow (possibly).

kre



Home | Main Index | Thread Index | Old Index