use strlcat() strlcpy() and slprintf() whenever possible to avoid any

chance of a buffer overflow

diff --git a/authenticate.c b/authenticate.c
index c3a3c18..351c8a0 100644 (file)
--- a/authenticate.c
+++ b/authenticate.c
@@ -28,7 +28,6 @@ static void base64_encode(char *buf, int len, char *out)
        char *b64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
        int bit_offset, byte_offset, idx, i;
        unsigned char *d = (unsigned char *)buf;
-       char *p;
        int bytes = (len*8 + 5)/6;
 
        memset(out, 0, bytes+1);
@@ -56,7 +55,7 @@ static void gen_challenge(char *addr, char *challenge)
 
        memset(input, 0, sizeof(input));
 
-       strncpy((char *)input, addr, 16);
+       strlcpy((char *)input, addr, 16);
        gettimeofday(&tv, NULL);
        SIVAL(input, 16, tv.tv_sec);
        SIVAL(input, 20, tv.tv_usec);
@@ -74,8 +73,8 @@ static int get_secret(int module, char *user, char *secret, int len)
 {
        char *fname = lp_secrets_file(module);
        int fd, found=0;
-       char line[1024];
-       char *p, *pass;
+       char line[MAXPATHLEN];
+       char *p, *pass=NULL;
 
        if (!fname || !*fname) return 0;
 
@@ -137,7 +136,7 @@ int auth_server(int fd, int module, char *addr, char *leader)
        char *users = lp_auth_users(module);
        char challenge[16];
        char b64_challenge[30];
-       char line[1024];
+       char line[MAXPATHLEN];
        char user[100];
        char secret[100];
        char pass[30];

diff --git a/clientserver.c b/clientserver.c
index b8c4da0..1c0bd7e 100644 (file)
--- a/clientserver.c
+++ b/clientserver.c
@@ -30,7 +30,7 @@ int start_socket_client(char *host, char *path, int argc, char *argv[])
        int fd, i;
        char *sargs[MAX_ARGS];
        int sargc=0;
-       char line[1024];
+       char line[MAXPATHLEN];
        char *p, *user=NULL;
        extern int remote_version;
 
@@ -102,13 +102,12 @@ static int rsync_module(int fd, int i)
        int argc=0;
        char *argv[MAX_ARGS];
        char **argp;
-       char line[1024];
+       char line[MAXPATHLEN];
        uid_t uid;
        gid_t gid;
        char *p;
        char *addr = client_addr(fd);
        char *host = client_name(fd);
-       char *auth;
        char *name = lp_name(i);
        int start_glob=0;
 
@@ -201,17 +200,13 @@ static int rsync_module(int fd, int i)
 
                p = line;
 
-               if (start_glob && strncmp(p, name, strlen(name)) == 0) {
-                       p += strlen(name);
-                       if (!*p) p = ".";
-               }
-
                argv[argc] = strdup(p);
                if (!argv[argc]) {
                        return -1;
                }
 
                if (start_glob) {
+                       rprintf(FINFO,"transferring %s\n",p);
                        glob_expand(name, argv, &argc, MAX_ARGS);
                } else {
                        argc++;
@@ -331,6 +326,8 @@ int daemon_main(void)
 
        become_daemon();
 
+       rprintf(FINFO,"rsyncd version %s starting\n",VERSION);
+
        start_accept_loop(rsync_port, start_daemon);
        return -1;
 }

diff --git a/exclude.c b/exclude.c
index d5c126f..901047e 100644 (file)
--- a/exclude.c
+++ b/exclude.c
@@ -205,8 +205,8 @@ void add_cvs_excludes(void)
     add_exclude(cvs_ignore_list[i]);
 
   if ((p=getenv("HOME")) && strlen(p) < (MAXPATHLEN-12)) {
-    sprintf(fname,"%s/.cvsignore",p);
-    add_exclude_file(fname,0);
+         slprintf(fname,sizeof(fname)-1, "%s/.cvsignore",p);
+         add_exclude_file(fname,0);
   }
 
   add_exclude_line(getenv("CVSIGNORE"));

diff --git a/flist.c b/flist.c
index d7da6e8..bc1ceba 100644 (file)
--- a/flist.c
+++ b/flist.c
@@ -521,7 +521,7 @@ static void send_directory(int f,struct file_list *flist,char *dir)
                        closedir(d);
                        return;
                }
-               strcat(fname,"/");
+               strlcat(fname,"/", MAXPATHLEN-1);
                l++;
        }
        p = fname + strlen(fname);
@@ -585,7 +585,7 @@ struct file_list *send_file_list(int f,int argc,char *argv[])
 
                l = strlen(fname);
                if (l != 1 && fname[l-1] == '/') {
-                       strcat(fname,".");
+                       strlcat(fname,".",MAXPATHLEN-1);
                }
 
                if (link_stat(fname,&st) != 0) {
@@ -878,7 +878,7 @@ char *f_name(struct file_struct *f)
        n = (n+1)%10;
 
        if (f->dirname) {
-               sprintf(p, "%s/%s", f->dirname, f->basename);
+               slprintf(p, MAXPATHLEN-1, "%s/%s", f->dirname, f->basename);
        } else {
                strlcpy(p, f->basename, MAXPATHLEN-1);
        }

diff --git a/io.c b/io.c
index 1fafaa6..ccb099a 100644 (file)
--- a/io.c
+++ b/io.c
@@ -521,13 +521,7 @@ void io_printf(int fd, const char *format, ...)
        int len;
        
        va_start(ap, format);
-
-#if HAVE_VSNPRINTF
-       len = vsnprintf(buf, sizeof(buf)-1, format, ap);
-#else
-       vsprintf(buf, format, ap);
-       len = strlen(buf);
-#endif
+       len = vslprintf(buf, sizeof(buf)-1, format, ap);
        va_end(ap);
 
        if (len < 0) exit_cleanup(1);

diff --git a/log.c b/log.c
index 068b5a9..8be3605 100644 (file)
--- a/log.c
+++ b/log.c
@@ -23,6 +23,7 @@
   */
 #include "rsync.h"
 
+
 /* this is the rsync debugging function. Call it with FINFO or FERROR */
 void rprintf(int fd, const char *format, ...)
 {
@@ -33,13 +34,7 @@ void rprintf(int fd, const char *format, ...)
        extern int am_daemon;
 
        va_start(ap, format);
-
-#if HAVE_VSNPRINTF
-       len = vsnprintf(buf, sizeof(buf)-1, format, ap);
-#else
-       vsprintf(buf, format, ap);
-       len = strlen(buf);
-#endif
+       len = vslprintf(buf, sizeof(buf)-1, format, ap);
        va_end(ap);
 
        if (len < 0) exit_cleanup(1);

diff --git a/main.c b/main.c
index f914f3e..6dfaf39 100644 (file)
--- a/main.c
+++ b/main.c
@@ -190,7 +190,6 @@ static void do_server_sender(int f_in, int f_out, int argc,char *argv[])
        struct file_list *flist;
        char *dir = argv[0];
        extern int relative_paths;
-       extern int am_daemon;
        extern int recurse;
 
        if (verbose > 2)

diff --git a/rsync.c b/rsync.c
index 98af1cd..a9c2438 100644 (file)
--- a/rsync.c
+++ b/rsync.c
@@ -111,10 +111,7 @@ static int delete_file(char *fname)
                if (strcmp(dname,".")==0 ||
                    strcmp(dname,"..")==0)
                        continue;
-               strlcpy(buf, fname, (MAXPATHLEN-strlen(dname))-2);
-               strcat(buf, "/");
-               strcat(buf, dname);
-               buf[MAXPATHLEN-1] = 0;
+               slprintf(buf, sizeof(buf)-1, "%s/%s", fname, dname);
                if (verbose > 0)
                        rprintf(FINFO,"deleting %s\n", buf);
                if (delete_file(buf) != 0) {
@@ -831,7 +828,7 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen)
                      close(fd1);
                      continue;
              }
-             sprintf(fnametmp,"%s/.%s.XXXXXX",tmpdir,f);
+             slprintf(fnametmp,sizeof(fnametmp)-1, "%s/.%s.XXXXXX",tmpdir,f);
       } else {
              char *f = strrchr(fname,'/');
 
@@ -844,10 +841,10 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen)
 
              if (f) {
                      *f = 0;
-                     sprintf(fnametmp,"%s/.%s.XXXXXX",fname,f+1);
+                     slprintf(fnametmp,sizeof(fnametmp)-1,"%s/.%s.XXXXXX",fname,f+1);
                      *f = '/';
              } else {
-                     sprintf(fnametmp,".%s.XXXXXX",fname);
+                     slprintf(fnametmp,sizeof(fnametmp)-1,".%s.XXXXXX",fname);
              }
       }
       if (NULL == do_mktemp(fnametmp)) {
@@ -893,7 +890,7 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen)
                rprintf(FERROR,"backup filename too long\n");
                continue;
        }
-       sprintf(fnamebak,"%s%s",fname,backup_suffix);
+       slprintf(fnamebak,sizeof(fnamebak)-1,"%s%s",fname,backup_suffix);
        if (do_rename(fname,fnamebak) != 0 && errno != ENOENT) {
          rprintf(FERROR,"rename %s %s : %s\n",fname,fnamebak,strerror(errno));
          continue;
@@ -998,10 +995,10 @@ void send_files(struct file_list *flist,int f_out,int f_in)
                                  fname);
                          return;
                  }
-                 strcat(fname,"/");
+                 strlcat(fname,"/",MAXPATHLEN-1);
                  offset = strlen(file->basedir)+1;
          }
-         strncat(fname,f_name(file),MAXPATHLEN-strlen(fname));
+         strlcat(fname,f_name(file),MAXPATHLEN-strlen(fname));
          
          if (verbose > 2) 
                  rprintf(FINFO,"send_files(%d,%s)\n",i,fname);

diff --git a/util.c b/util.c
index 1fdeee6..cc39cdb 100644 (file)
--- a/util.c
+++ b/util.c
@@ -456,6 +456,21 @@ void strlcpy(char *d, char *s, int maxlen)
        d[len] = 0;
 }
 
+/* like strncat but does not 0 fill the buffer and always null 
+   terminates (thus it can use maxlen+1 space in d) */
+void strlcat(char *d, char *s, int maxlen)
+{
+       int len1 = strlen(d);
+       int len2 = strlen(s);
+       if (len1+len2 > maxlen) {
+               len2 = maxlen-len1;
+       }
+       if (len2 > 0) {
+               memcpy(d+len1, s, len2);
+               d[len1+len2] = 0;
+       }
+}
+
 /* turn a user name into a uid */
 int name_to_uid(char *name, uid_t *uid)
 {
@@ -509,6 +524,7 @@ int lock_range(int fd, int offset, int len)
 static void glob_expand_one(char *s, char **argv, int *argc, int maxargs)
 {
 #ifndef HAVE_GLOB
+       if (!*s) s = ".";
        argv[*argc] = strdup(s);
        (*argc)++;
        return;
@@ -516,6 +532,8 @@ static void glob_expand_one(char *s, char **argv, int *argc, int maxargs)
        glob_t globbuf;
        int i;
 
+       if (!*s) s = ".";
+
        argv[*argc] = strdup(s);
 
        memset(&globbuf, 0, sizeof(globbuf));
@@ -542,6 +560,10 @@ void glob_expand(char *base, char **argv, int *argc, int maxargs)
 
        if (!s || !*s) return;
 
+       if (strncmp(s, base, strlen(base)) == 0) {
+               s += strlen(base);
+       }
+
        s = strdup(s);
        if (!s) out_of_memory("glob_expand");
 
@@ -551,7 +573,7 @@ void glob_expand(char *base, char **argv, int *argc, int maxargs)
                        /* split it at this point */
                        *(p-1) = 0;
                        glob_expand_one(q, argv, argc, maxargs);
-                       q = p+strlen(base);
+                       q = p+strlen(base)+1;
                } else {
                        q++;
                }
@@ -572,3 +594,74 @@ void strlower(char *s)
                s++;
        }
 }
+
+/* this is like vsnprintf but the 'n' limit does not include
+   the terminating null. So if you have a 1024 byte buffer then
+   pass 1023 for n */
+int vslprintf(char *str, int n, const char *format, va_list ap)
+{
+#ifdef HAVE_VSNPRINTF
+       int ret = vsnprintf(str, n, format, ap);
+       if (ret > n || ret < 0) {
+               str[n] = 0;
+               return -1;
+       }
+       str[ret] = 0;
+       return ret;
+#else
+       static char *buf;
+       static int len=MAXPATHLEN*8;
+       int ret;
+
+       /* this code is NOT a proper vsnprintf() implementation. It
+          relies on the fact that all calls to slprintf() in rsync
+          pass strings which have already been checked to be less
+          than MAXPATHLEN in length and never more than 2 strings are
+          concatenated. This means the above buffer is absolutely
+          ample and can never be overflowed.
+
+          In the future we would like to replace this with a proper
+          vsnprintf() implementation but right now we need a solution
+          that is secure and portable. This is it.  */
+
+       if (!buf) {
+               buf = malloc(len);
+               if (!buf) {
+                       /* can't call debug or we would recurse */
+                       exit(1);
+               }
+       }
+
+       ret = vsprintf(buf, format, ap);
+
+       if (ret < 0) {
+               str[0] = 0;
+               return -1;
+       }
+
+       if (ret < n) {
+               n = ret;
+       } else if (ret > n) {
+               ret = -1;
+       }
+
+       buf[n] = 0;
+       
+       memcpy(str, buf, n+1);
+
+       return ret;
+#endif
+}
+
+
+/* like snprintf but always null terminates */
+int slprintf(char *str, int n, char *format, ...)
+{
+       va_list ap;  
+       int ret;
+
+       va_start(ap, format);
+       ret = vslprintf(str,n,format,ap);
+       va_end(ap);
+       return ret;
+}