defence-in-depth: guard cumulative snprintf against length underflow

Two cumulative-snprintf patterns in log.c (rsyserr) and main.c
(output_itemized_counts) had the shape

    len = snprintf(buf, sizeof buf, ...);
    len += snprintf(buf+len, sizeof buf - len, ...);

with no guard between calls. snprintf returns the would-have-been
length on truncation, so a truncated first call leaves
"sizeof buf - len" as a negative-then-promoted-to-size_t value,
underflowing into a huge size_t and writing past buf.

Realistic exposure is small in both cases (log header well under
buffer, only ~5 itemized iterations writing ~25 chars each into a
1024-byte buffer) but the defect class matches bb0a8118 and the
fix is cheap. Guard before each subsequent call.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

diff --git a/log.c b/log.c
index e4ba1cce24122c62ee21cdb9ed5b550993131cea..b948f16a1013d691408d72ed27241fef4fe31b63 100644 (file)
--- a/log.c
+++ b/log.c
@@ -456,11 +456,17 @@ void rsyserr(enum logcode code, int errcode, const char *format, ...)
        char buf[BIGPATHBUFLEN];
        size_t len;
 
+       /* snprintf returns the would-have-been length on truncation, so
+        * each cumulative call must be guarded; if not, sizeof buf - len
+        * can underflow when promoted to size_t and the next call writes
+        * past the buffer. */
        len = snprintf(buf, sizeof buf, RSYNC_NAME ": [%s] ", who_am_i());
 
-       va_start(ap, format);
-       len += vsnprintf(buf + len, sizeof buf - len, format, ap);
-       va_end(ap);
+       if (len < sizeof buf) {
+               va_start(ap, format);
+               len += vsnprintf(buf + len, sizeof buf - len, format, ap);
+               va_end(ap);
+       }
 
        if (len < sizeof buf) {
                len += snprintf(buf + len, sizeof buf - len,

diff --git a/main.c b/main.c
index 6347934e2e97c8636455afe96dad7fd6703e5573..8c6718eb3130594f74327194f0809f32880002be 100644 (file)
--- a/main.c
+++ b/main.c
@@ -394,9 +394,18 @@ static void output_itemized_counts(const char *prefix, int *counts)
                counts[0] -= counts[1] + counts[2] + counts[3] + counts[4];
                for (j = 0; j < 5; j++) {
                        if (counts[j]) {
+                               /* snprintf can return more than its size arg
+                                * on truncation; keep len <= sizeof buf - 2 so
+                                * the closing ')' and trailing NUL always
+                                * have room and the next iteration's
+                                * sizeof buf - len - 2 cannot underflow. */
+                               if (len >= (int)sizeof buf - 2)
+                                       break;
                                len += snprintf(buf+len, sizeof buf - len - 2,
                                        "%s%s: %s",
                                        pre, labels[j], comma_num(counts[j]));
+                               if (len > (int)sizeof buf - 2)
+                                       len = (int)sizeof buf - 2;
                                pre = ", ";
                        }
                }