From e42c9458c2f1e3a78d6d45e99741d6edb38fc0cc Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 15 May 1998 09:26:01 +0000 Subject: [PATCH] use strlcat() strlcpy() and slprintf() whenever possible to avoid any chance of a buffer overflow --- authenticate.c | 9 +++-- clientserver.c | 13 +++---- exclude.c | 4 +-- flist.c | 6 ++-- io.c | 8 +---- log.c | 9 ++--- main.c | 1 - rsync.c | 17 ++++----- util.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 9 files changed, 118 insertions(+), 44 deletions(-) diff --git a/authenticate.c b/authenticate.c index c3a3c184..351c8a0a 100644 --- 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 b8c4da04..1c0bd7ec 100644 --- 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 d5c126fe..901047eb 100644 --- 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 d7da6e88..bc1ceba9 100644 --- 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 1fafaa62..ccb099a9 100644 --- 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 068b5a93..8be3605a 100644 --- 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 f914f3e4..6dfaf396 100644 --- 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 98af1cda..a9c24387 100644 --- 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 1fdeee6f..cc39cdb4 100644 --- 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; +} -- 2.34.1