Ushering out strlcpy()

Posted Sep 2, 2022 11:16 UTC (Fri) by mtodorov (guest, #158788)
Parent article: Ushering out strlcpy()

Just occurred to me, maybe it is a stupid idea ...

What if strlcpy() was fixed instead of changing 1,000s of occurrences?

Changing the strlen(src) to strnlen(src, size) should suffice. This would both prevent segfaults for long unterminated strings and preserve function semantics.

The fix is simple:

    size_t strlcpy(char *dest, const char *src, size_t size)
    {
	size_t ret = strnlen(src, size);

	if (size) {
	    size_t len = (ret >= size) ? size - 1 : ret;
	    memcpy(dest, src, len);
	    dest[len] = '\0';
	}
	return ret;
    }

Am I making any sense to you? strnlen (src, size) is said to be POSIX-2008 compliant ...

to post comments

Ushering out strlcpy()

Posted Sep 2, 2022 12:08 UTC (Fri) by mathstuf (subscriber, #69389) [Link] (3 responses)

You've broken compatibility for callers which do something like:

```
size_t ret = bufsz;
size_t sz = bufsz;
while (sz <= ret) {
ret = strlcpy(buf, in, bufsz);
if (sz <= ret) {
buf = realloc(buf, ret);
sz = ret;
}
}
```

to 1-stepping the buffer increment instead of just doing a single realloc-enough loop. The return value is "the size that would have been attempted", not "something bigger than".

Ushering out strlcpy()

Posted Sep 2, 2022 13:33 UTC (Fri) by mtodorov (guest, #158788) [Link] (2 responses)

I see what you're trying to do. My presumption was obviously wrong: changing the return value amounts to a change of semantics, which makes the premise false. And requires another function not to break the existing sources.

Yes, I see indeed. Anyway, it seemed too simple and obvious so no one wouldn't have thought of it earlier.

Thanks, @mathstuf.

However, I believe that attempting and unbound strlen(src) on a fixed-size buffer is inherently and semantically wrong, as the size can never be greater than the size of the buffer unless you're performing a scan beyond buffer's end on a potentially unfinished string, which as Mr. Torvalds said can bring disaster (SEGFAULT in kernel mode).

strlen (buf) should thereof always be done as strnlen (buf, sizeof (buf)). Beyond the end of the buffer there can be beasts, nasty animals and hidden pits.

IMHO.

Ushering out strlcpy()

Posted Sep 2, 2022 13:38 UTC (Fri) by mathstuf (subscriber, #69389) [Link] (1 responses)

> strlen (buf) should thereof always be done as strnlen (buf, sizeof (buf)). Beyond the end of the buffer there can be beasts, nasty animals and hidden pits.

The problem is that `sizeof (buf)` is not known in a lot of contexts (e.g., filenames passed in from userspace). At that point, you're starting to carry around the allocated size around and you may as well amortize the length calculation as well…and oh, what do you know, you have a C++ std::string with no relevant established API.

Ushering out strlcpy()

Posted Sep 2, 2022 13:50 UTC (Fri) by mtodorov (guest, #158788) [Link]

> The problem is that `sizeof (buf)` is not known in a lot of contexts (e.g., filenames passed in from userspace). At that point, you're starting to carry around the allocated size around and you may as well amortize the length calculation as well…and oh, what do you know, you have a C++ std::string with no relevant established API.

No argument on that.
However, in most cases there seems to be an implementation-specific limit, like `strnlen (buf, MAXPATHLEN)`.
Attempting to process more than that which makes sense usually only brings a risk of buffer overflow anyway. IMHO.
Carefully crafted input could place the first NULL byte at the beginning of the next memory page that is not allocated and trigger SIGBUS. Probably difficult to exploit, but still possible?