summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlejandro Colomar <alx@kernel.org>2023-11-12 13:43:32 +0100
committerAlejandro Colomar <alx@kernel.org>2023-11-16 11:44:25 +0100
commita28abcc38bd628ca643fa650e7ebfee38ef648b9 (patch)
treee320e63932ca4ec026df9be258a0578f44f9067d
parent3051205f1e10b4f4e3217f661d776d3fdde117bd (diff)
lib/strlcpy.[ch]: Implement strtcpy(3) to replace strlcpy_()
There's been a very long and interesting discussion in linux-man@ and libc-alpha@, where we've discussed all the string-copying functions, their pros and cons, when should each be used and avoided, etc. Paul Eggert pointed out an important problem of strlcpy(3): it is vulnerable to DoS attacks if an attacker controls the length of the source string. And even if it doesn't control it, the function is dead slow (because its API forces it to calculate strlen(src)). We've agreed that the general solution for a truncating string-copying function is to write a wrapper over strnlen(3)+memcpy(3), which is limited to strnlen(src, sizeof(dst)). This is not vulnerable to DoS, and is very fast for all buffer sizes. string_copying(7) has been updated to reflect this, and provides a reference implementation for this wrapper function. This strtcpy(3) (t for truncation) wrapper happens to have the same API that our strlcpy_() function had, so replace it with the better implementation. We don't need to update callers nor tests, since the API is the same. A future commit will rename STRLCPY() to STRTCPY(), and replace remaining calls to strlcpy(3) by calls to this strtcpy(3). Link: <https://lore.kernel.org/linux-man/ZU4SDh-Se5gjPny5@debian/T/#mfb5a3fdeb35487dec6f8d9e3d8548bd0d92c4975/> Signed-off-by: Alejandro Colomar <alx@kernel.org>
-rw-r--r--lib/strlcpy.c4
-rw-r--r--lib/strlcpy.h25
-rw-r--r--tests/unit/Makefile.am2
3 files changed, 20 insertions, 11 deletions
diff --git a/lib/strlcpy.c b/lib/strlcpy.c
index ffb83e0c..58773b71 100644
--- a/lib/strlcpy.c
+++ b/lib/strlcpy.c
@@ -14,5 +14,5 @@
#include "strlcpy.h"
-extern inline ssize_t strlcpy_(char *restrict dst, const char *restrict src,
- size_t size);
+extern inline ssize_t strtcpy(char *restrict dst, const char *restrict src,
+ size_t dsize);
diff --git a/lib/strlcpy.h b/lib/strlcpy.h
index c44819c6..36d847c4 100644
--- a/lib/strlcpy.h
+++ b/lib/strlcpy.h
@@ -10,6 +10,7 @@
#include <config.h>
+#include <stdbool.h>
#include <stddef.h>
#include <string.h>
#include <sys/types.h>
@@ -43,21 +44,31 @@
*/
-#define STRLCPY(dst, src) strlcpy_(dst, src, SIZEOF_ARRAY(dst))
+#define STRLCPY(dst, src) strtcpy(dst, src, SIZEOF_ARRAY(dst))
-inline ssize_t strlcpy_(char *restrict dst, const char *restrict src,
- size_t size);
+inline ssize_t strtcpy(char *restrict dst, const char *restrict src,
+ size_t dsize);
inline ssize_t
-strlcpy_(char *restrict dst, const char *restrict src, size_t size)
+strtcpy(char *restrict dst, const char *restrict src, size_t dsize)
{
- size_t len;
+ bool trunc;
+ char *p;
+ size_t dlen, slen;
- len = strlcpy(dst, src, size);
+ if (dsize == 0)
+ return -1;
- return (len >= size) ? -1 : len;
+ slen = strnlen(src, dsize);
+ trunc = (slen == dsize);
+ dlen = slen - trunc;
+
+ p = mempcpy(dst, src, dlen);
+ *p = '\0';
+
+ return trunc ? -1 : slen;
}
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index e209f041..46794d66 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -38,13 +38,11 @@ test_strlcpy_SOURCES = \
$(NULL)
test_strlcpy_CFLAGS = \
$(AM_FLAGS) \
- $(LIBBSD_CFLAGS) \
$(NULL)
test_strlcpy_LDFLAGS = \
$(NULL)
test_strlcpy_LDADD = \
$(CMOCKA_LIBS) \
- $(LIBBSD_LIBS) \
$(NULL)
test_xasprintf_SOURCES = \