On 10/11/2023 13:15, Alejandro Colomar wrote:
> 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.
That is good, E2BIG if the dest_max_size can't accommodate src_max_size
>
> 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.
In the past I've used different values, eg -1 .. -5 as there are 5 different errors detected by this function I made a test version of, so application just needs to check for 0 for success. (The different error returns are useful when the issue is logged, to see where the error was detected in the function.)
> - 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.
The application should know the src buffer size, given that it allocated the buffer. That saves the performance cost of strnlen().
> 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.
Sounds good.
>
> - 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
>
Kind regards, Jonny
