]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
log: copy forwarded message by length in rwrite(), not strlcpy()
authorAndrew Tridgell <andrew@tridgell.net>
Sat, 13 Jun 2026 08:12:07 +0000 (18:12 +1000)
committerAndrew Tridgell <andrew@tridgell.net>
Sat, 13 Jun 2026 08:56:32 +0000 (18:56 +1000)
The valgrind memcheck CI flagged 'Conditional jump depends on uninitialised
value(s)' in rwrite() -> strlcpy() (log.c) and the subsequent logit() fprintf.
rwrite()'s daemon/logfile branch did strlcpy(msg, buf, MIN(sizeof msg, len+1)),
but strlcpy() scans the whole source with strlen(); buf is the data buffer from
read_a_msg() (io.c) holding exactly len bytes of a forwarded MSG_* payload with
no NUL terminator, so strlen() reads past the message into uninitialised stack.

Copy exactly len (bounded) bytes with memcpy() and NUL-terminate, matching the
(buf, len) contract the rest of rwrite() already honours.  Behaviour is
unchanged for the NUL-terminated callers; the over-read is gone.

Full testsuite under valgrind (1572 logs) now reports zero unsuppressed errors.

log.c

diff --git a/log.c b/log.c
index 5f1f3a29ca61151e2ab38ce7d29b42abb15fcad5..3e9a2edcfca94ae4cde3b8faa07660d275f29c50 100644 (file)
--- a/log.c
+++ b/log.c
@@ -298,7 +298,12 @@ void rwrite(enum logcode code, const char *buf, int len, int is_utf8)
                in_block = 1;
                if (!log_initialised)
                        log_init(0);
-               strlcpy(msg, buf, MIN((int)sizeof msg, len + 1));
+               /* buf holds exactly len bytes and is not necessarily NUL-terminated
+                * (e.g. a forwarded MSG_* payload from read_a_msg), so copy by length
+                * rather than strlcpy(), which would strlen() past the end of buf. */
+               int mlen = MIN((int)sizeof msg - 1, len);
+               memcpy(msg, buf, mlen);
+               msg[mlen] = '\0';
                logit(priority, msg);
                in_block = 0;