[RFC] strcpys(): New function for copying strings safely

* [RFC] strcpys(): New function for copying strings safely
@ 2021-06-27 19:26 Alejandro Colomar (man-pages)
  2021-06-27 19:46 ` Alejandro Colomar (man-pages)
  0 siblings, 1 reply; 6+ messages in thread
From: Alejandro Colomar (man-pages) @ 2021-06-27 19:26 UTC (permalink / raw)
  To: glibc; +Cc: tech, Christoph Hellwig, linux-kernel

Hi,

I recently re-read the discussions about strscpy() and strlcpy().

https://lwn.net/Articles/612244/
https://sourceware.org/legacy-ml/libc-alpha/2000-08/msg00052.html
https://mafford.com/text/the-many-ways-to-copy-a-string-in-c/
https://lkml.org/lkml/2017/7/14/637
https://lwn.net/Articles/659214/

Arguments against strlcpy():

- Inefficiency (compared to memcpy())
- It doesn't report truncation, needing extra checks by the user
- It doesn't prevent overrunning the source string (requires a C-string 
input)

Arguments in favor of strlcpy():

- Avoid code bloat
- Self-documenting code
- Avoid everybody rolling their own strcat() safe variant
- Prevent buffer overflows

I rolled my own strcpy safer functions some time ago, and after reading 
those discussions, I decided to propose them for inclusion in glibc.

I think they address all of the arguments before (well, efficiency will 
never be as good as memcpy(), I guess, but a good compiler, and -flto 
can make it good enough).

It reports two kinds of errors: "hard" errors, where the string is 
invalid, and "soft" errors, where the string is valid but truncated.
The method for reporting the errors is an 'int' return value, which is 0 
for success, -1 for "hard" error, and 1 for "soft" error.

The value of written characters that strcpy() functions typically 
return, is now moved to a pointer parameter (4th parameter).

The order of the parameters has been changed (compared to other strcpy() 
typicall variants), according to a new principle of the C2x standard, 
which says that "the size of an array appears before the array".
<http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2086.htm>

It doesn't require a C string in the input.  It only reads up to the 
size limit provided by the user.

I added the _np suffix to the function to mark it as non-portable 
(similar to existing practice in glibc with non-standard functions).

I used 'ssize_t' instead of 'size_t', because I consider unsigned types 
to be inherently unsafe.  See 
<https://google.github.io/styleguide/cppguide.html#Integer_Types>.

It is designed so that usage requires the minimum number of lines of 
code for complete usage (including error handling checks):

[[
// When we already checked that 'size' is >= 1
// and truncation is not an issue:

strcpys_np(size, dest, src, NULL);

// When we ddidn't check the value of 'size',
// but truncation is still not an issue:

if (strcpys_np(size, dest, src, NULL) == -1)
	goto handle_hard_error;

// When truncation is an issue:

if (strcpys_np(size, dest, src, NULL))
	goto handle_all_errors;

// When truncation is an error,
// but it requires a different handling than a "hard" error:

status = strcpys_np(size, dest, src, NULL);
if (status == 1)
	goto handle_truncation;
if (status)
	goto handle_hard_error;
]]

After any of those samples of code, the string has been copied, and is a 
valid C-string.

Here goes the code (strcpys_np() is defined in terms of strscpy_np(), 
which similar to the known strscpy(), but with some of the improvements 
mentioned above, such as using array parameters, and ssize_t):

[[

#include <string.h>
#include <sys/types.h>

[[gnu::nonnull]]
ssize_t strscpy_np(ssize_t size,
                    char dest[static restrict size],
                    const char src[static restrict size])
{
	ssize_t len;

	if (size <= 0)
		return -1;

	len = strnlen(src, size - 1);
	memcpy(dest, src, len);
	dest[len] = '\0';

	return len;
}

[[gnu::nonnull(2, 3)]]
int strcpys_np(ssize_t size,
                char dest[static restrict size],
                const char src[static restrict size],
                ssize_t *restrict len)
{
	ssize_t l;

	l = strscpy_np(size, dest, src);
	if (len)
		*len = l;

	if (l == -1)
		return -1;
	if (l >= size)
		return 1;
	return 0;
}

]]

I may have introduced some bugs right now, as I adapted the code a bit 
before sending, but I expect it to be free of any bugs known of the 
existing str*cpy() interfaces.

What do you think about this function?  Would you want to add it to glibc?

Thanks,

Alex

--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

^ permalink raw reply	[flat|nested] 6+ messages in thread