Re: strncpy clarify result may not be null terminated

Alejandro Colomar
2023-11-10T17:48:48Z

[-- Attachment #1: Type: text/plain, Size: 3255 bytes --]

Hi Jonny,

On Fri, Nov 10, 2023 at 11:36:20AM +0000, Jonny Grant wrote:
>
>
> On 10/11/2023 05:36, Paul Eggert wrote:
> > On 2023-11-09 15:48, Alejandro Colomar wrote:
> >> I'd then just use strlen(3)+strcpy(3), avoiding
> >> strncpy(3).
> >
> > But that is vulnerable to the same denial-of-service attack that strlcpy is vulnerable to. You'd need strnlen+strcpy instead.
> >
> > The strncpy approach I suggested is simpler, and (though this doesn't matter much in practice) is typically significantly faster than strnlen+strcpy in the typical case where the destination is a small fixed-size buffer.
> >
> > Although strncpy is not a good design, it's often simpler or faster or safer than later "improvements".
>
> As you say, it is a known API. I recall looking for a standardized bounded string copy a few years ago that avoids pitfalls:
>
> 1) cost of any initial strnlen() reading memory to determine input src size
> 2) accepts a src_max_size to actually try to copy from src
> 3) does not truncate by writing anything to the buffer if there isn't enough space in the dest_max_size to fit src_max_size
> 4) check for NULL pointers
> 5) probably other thing I've overlooked
>
> Something like this API:
> int my_str_copy(char *dest, const char *src, size_t dest_max_size, size_t src_max_size, size_t * dest_written);
> These sizes are including any NUL terminating byte.
>
> 0 on success, or an an error code like EINVAL, or ERANGE if would truncate

- Linux kernel's strscpy() returns -E2BIG if it would truncate. You
may want to follow suit if you want such an errno(3) code.

However, I think it's simpler to return the "standard" user-space
error return value: -1

If you'd need to distinguish error reasons, you could distinguish
error codes, but for a string-copying function I think it's not so
useful.

- Why specify the src buffer size? If you're copying strings, then you
know it'll be null-terminated, so strnlen(3) will not overrun. If
you're not copying strings, then you'll need a different function
that reads from a non-string. The only standard such function is
strncat(3), which reads from a fixed-width null-padded buffer, and
writes to a string. You may want to write a function similar to
strncat(3) that doesn't catenate, if you want to just copy; I call
that function zustr2stp(), and you can find an implementation in
string_copying(7).

- You can reuse the return value for the dest_written value with
ssize_t. Just return -1 on error and the string length on success.
That's how most libc functions behave.

- Regarding NULL checks, it depends on how you program. I wouldn't add
them, but if you want to avoid crashes at all costs, it may be
necessary for you. You could do a wrapper over strxcpy():

inline ssize_t
strxcpy0(char *restrict dst, const char *restrict src, size_t dsize)
{
if (dst == NULL || src == NULL)
return -1;

return strxcpy(dst, src, dsize);
}

I used 0 in the name to mark that this function checks for null
pointers.

Cheers,
Alex

>
> All comments welcome.
>
> Kind regards, Jonny

--
<https://www.alejandro-colomar.es/>