the signed/unsigned change seems to have caused a logic bug on some

[rsync/rsync.git] / log.c

diff --git a/log.c b/log.c
index 3b53a9a..045f031 100644 (file)
--- a/log.c
+++ b/log.c
@@ -286,13 +286,14 @@ void rprintf(enum logcode code, const char *format, ...)
        int len;
 
        va_start(ap, format);
+       /* Note: might return -1 */
        len = vsnprintf(buf, sizeof(buf), format, ap);
        va_end(ap);
 
        /* Deal with buffer overruns.  Instead of panicking, just
         * truncate the resulting string.  Note that some vsnprintf()s
         * return -1 on truncation, e.g., glibc 2.0.6 and earlier. */
-       if (len > sizeof(buf)-1  ||  len < 0) {
+       if ((size_t) len > sizeof(buf)-1  ||  len < 0) {
                const char ellipsis[] = "[...]";
 
                /* Reset length, and zero-terminate the end of our buffer */
@@ -331,18 +332,21 @@ void rsyserr(enum logcode code, int errcode, const char *format, ...)
 {
        va_list ap;  
        char buf[1024];
-       int len, sys_len;
+       int len;
+       size_t sys_len;
         char *sysmsg;
 
        va_start(ap, format);
+       /* Note: might return <0 */
        len = vsnprintf(buf, sizeof(buf), format, ap);
        va_end(ap);
 
-       if (len > sizeof(buf)-1) exit_cleanup(RERR_MESSAGEIO);
+       if ((size_t) len > sizeof(buf)-1)
+               exit_cleanup(RERR_MESSAGEIO);
 
         sysmsg = strerror(errcode);
         sys_len = strlen(sysmsg);
-        if (len + 3 + sys_len > sizeof(buf) - 1)
+        if ((size_t) len + 3 + sys_len > sizeof(buf) - 1)
                 exit_cleanup(RERR_MESSAGEIO);
 
         strcpy(buf + len, ": ");
@@ -399,12 +403,18 @@ static void log_formatted(enum logcode code,
        char buf[1024];
        char buf2[1024];
        char *p, *s, *n;
-       int l;
+       size_t l;
        extern struct stats stats;              
        extern int am_sender;
        extern int am_daemon;
        int64 b;
 
+       /* We expand % codes one by one in place in buf.  We don't
+        * copy in the terminating nul of the inserted strings, but
+        * rather keep going until we reach the nul of the format.
+        * Just to make sure we don't clobber that nul and therefore
+        * accidentally keep going, we zero the buffer now. */
+       memset(buf, 0, sizeof buf);
        strlcpy(buf, format, sizeof(buf));
        
        for (s=&buf[0]; 
@@ -462,21 +472,30 @@ static void log_formatted(enum logcode code,
                        break;
                }
 
-               if (!n) continue;
+               /* n is the string to be inserted in place of this %
+                * code; l is its length not including the trailing
+                * NUL */
+               if (!n)
+                       continue;
 
                l = strlen(n);
 
-               if ((l-1) + ((int)(s - &buf[0])) > sizeof(buf)) {
+               if (l + ((int)(s - &buf[0])) >= sizeof(buf)) {
                        rprintf(FERROR,"buffer overflow expanding %%%c - exiting\n",
                                p[0]);
                        exit_cleanup(RERR_MESSAGEIO);
                }
 
+               /* Shuffle the rest of the string along to make space for n */
                if (l != 2) {
                        memmove(s+(l-1), s+1, strlen(s+1)+1);
                }
+
+               /* Copy in n but NOT its nul, because the format sting
+                * probably continues after this. */
                memcpy(p, n, l);
 
+               /* Skip over inserted string; continue looking */
                s = p+l;
        }