netdev - [iproute PATCH] utils: Review strlcpy() and strlcat()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed,  6 Sep 2017 18:51:42 +0200
From:   Phil Sutter <phil@....cc>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org
Subject: [iproute PATCH] utils: Review strlcpy() and strlcat()

As David Laight correctly pointed out, the first version of strlcpy()
modified dst buffer behind the string copied into it. Fix this by
writing NUL to the byte immediately following src string instead of to
the last byte in dst. Doing so also allows to reduce overhead by using
memcpy().

Improve strlcat() by avoiding the call to strlcpy() if dst string is
already full, not just as sanity check.

Signed-off-by: Phil Sutter <phil@....cc>
---
 lib/utils.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 330ab073c2068..bbd3cbc46a0e5 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1233,18 +1233,22 @@ int get_real_family(int rtm_type, int rtm_family)
 
 size_t strlcpy(char *dst, const char *src, size_t size)
 {
+	size_t srclen = strlen(src);
+
 	if (size) {
-		strncpy(dst, src, size - 1);
-		dst[size - 1] = '\0';
+		size_t minlen = min(srclen, size - 1);
+
+		memcpy(dst, src, minlen);
+		dst[minlen] = '\0';
 	}
-	return strlen(src);
+	return srclen;
 }
 
 size_t strlcat(char *dst, const char *src, size_t size)
 {
 	size_t dlen = strlen(dst);
 
-	if (dlen > size)
+	if (dlen >= size)
 		return dlen + strlen(src);
 
 	return dlen + strlcpy(dst + dlen, src, size - dlen);
-- 
2.13.1