From a87d4d11f67d25a2abf16adc4d411c2811856aac Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fr=C3=A9d=C3=A9ric=20Marchal?= Date: Sun, 30 Oct 2011 19:04:12 +0000 Subject: [PATCH] Use a function to safely copy the strings Some calls already used strncpy to copy strings but, now, we are using a function to encapsulate the code and it is used to copy the arguments passed to sarg by the user. --- btree_cache.c | 4 ++-- email.c | 22 +++++++++++----------- exclude.c | 3 +-- getconf.c | 4 ++-- include/defs.h | 1 + index.c | 2 +- ip2name.c | 6 ++---- log.c | 28 +++++++++++++--------------- sort.c | 3 +-- userinfo.c | 3 +-- usertab.c | 22 ++++++++-------------- util.c | 25 +++++++++++++++++++++---- 12 files changed, 64 insertions(+), 59 deletions(-) diff --git a/btree_cache.c b/btree_cache.c index 995e6d4..b58a1a3 100644 --- a/btree_cache.c +++ b/btree_cache.c @@ -56,8 +56,8 @@ struct bt *insert_node(struct bt *root, const char *item, const char *value) new_item_bt = malloc(sizeof_bt); new_item_bt->left = NULL; new_item_bt->right = NULL; - strncpy(new_item_bt->value, item, 64); - strncpy(new_item_bt->targetattr, value, 256); + safe_strcpy(new_item_bt->value, item, sizeof(new_item_bt->value)); + safe_strcpy(new_item_bt->targetattr, value, sizeof(new_item_bt->targetattr)); new_item_bt->balanceinfo = 0; return new_item_bt; } diff --git a/email.c b/email.c index fec895c..d034b77 100644 --- a/email.c +++ b/email.c @@ -158,31 +158,31 @@ int geramail(const char *dirname, int debug, const char *outdir, const char *ema exit(EXIT_FAILURE); } - strcpy(strip1,_("Squid User Access Report")); + safe_strcpy(strip1,_("Squid User Access Report"),sizeof(strip1)); strip_latin(strip1); fprintf(fp_top3,"%s\n",strip1); - strcpy(strip1,_("Decreasing Access (bytes)")); + safe_strcpy(strip1,_("Decreasing Access (bytes)"),sizeof(strip1)); strip_latin(strip1); fprintf(fp_top3,"%s\n",strip1); - strcpy(strip1,_("Period")); + safe_strcpy(strip1,_("Period"),sizeof(strip1)); strip_latin(strip1); fprintf(fp_top3,"%s %s\n\n",strip1,period.text); - strcpy(strip1,_("NUM")); + safe_strcpy(strip1,_("NUM"),sizeof(strip1)); strip_latin(strip1); - strcpy(strip2,_("USERID")); + safe_strcpy(strip2,_("USERID"),sizeof(strip2)); strip_latin(strip2); - strcpy(strip3,_("CONNECT")); + safe_strcpy(strip3,_("CONNECT"),sizeof(strip3)); strip_latin(strip3); - strcpy(strip4,_("BYTES")); + safe_strcpy(strip4,_("BYTES"),sizeof(strip4)); strip_latin(strip4); - strcpy(strip5,_("ELAPSED TIME")); + safe_strcpy(strip5,_("ELAPSED TIME"),sizeof(strip5)); strip_latin(strip5); - strcpy(strip6,_("MILLISEC")); + safe_strcpy(strip6,_("MILLISEC"),sizeof(strip6)); strip_latin(strip6); - strcpy(strip7,_("TIME")); + safe_strcpy(strip7,_("TIME"),sizeof(strip7)); strip_latin(strip7); fprintf(fp_top3,"%-7s %-20s %-8s %-15s %%%-6s %-10s %-10s %%%-7s\n------- -------------------- -------- --------------- ------- ---------- ---------- -------\n",strip1,strip2,strip3,strip4,strip4,strip5,strip6,strip7); @@ -244,7 +244,7 @@ int geramail(const char *dirname, int debug, const char *outdir, const char *ema avgelap=0; } - strcpy(strip1,_("AVERAGE")); + safe_strcpy(strip1,_("AVERAGE"),sizeof(strip1)); strip_latin(strip1); #if defined(__FreeBSD__) fprintf(fp_top3,"%-7s %20s %8qu %15s %8s %9s %10qu\n",strip1," ",avgacc,fixnum(tnbytes,1)," ",buildtime(avgelap),avgelap); diff --git a/exclude.c b/exclude.c index a37edf9..80ff91f 100644 --- a/exclude.c +++ b/exclude.c @@ -171,8 +171,7 @@ static void store_exclude_url(const char *url,const char *next) debuga(_("Not enough memory to store the excluded URLs\n")); exit(EXIT_FAILURE); } - strncpy(item->url,url,length); - item->url[length]='\0'; + safe_strcpy(item->url,url,length+1); item->ndots=(ndots>0) ? ndots : -1; } diff --git a/getconf.c b/getconf.c index 549d6ff..e4326c5 100644 --- a/getconf.c +++ b/getconf.c @@ -655,9 +655,9 @@ static void parmtest(char *buf) debuga(_("Template file name is too long in parameter \"AuthUserTemplateFile\"\n")); exit(EXIT_FAILURE); } - strcpy(AuthUserTemplateFile,wbuf); + safe_strcpy(AuthUserTemplateFile,wbuf,sizeof(AuthUserTemplateFile)); } else { - strcpy(dir,ConfigFile); + safe_strcpy(dir,ConfigFile,sizeof(dir)); if (snprintf(AuthUserTemplateFile,sizeof(AuthUserTemplateFile),"%s/%s",dirname(dir),wbuf)>=sizeof(AuthUserTemplateFile)) { debuga(_("Template file name is too long in parameter \"AuthUserTemplateFile\"\n")); exit(EXIT_FAILURE); diff --git a/include/defs.h b/include/defs.h index 9ddd29d..d113468 100755 --- a/include/defs.h +++ b/include/defs.h @@ -253,6 +253,7 @@ void my_lltoa(unsigned long long int n, char *s, int ssize, int len); char *get_size(const char *path, const char *file); void url_module(const char *url, char *w2); void url_to_file(const char *url,char *file,int filesize); +void safe_strcpy(char *dest,const char *src,int length); void strip_latin(char *line); char *buildtime(long long int elap); int obtdate(const char *dirname, const char *name, char *data); diff --git a/index.c b/index.c index 651ef1b..c729080 100644 --- a/index.c +++ b/index.c @@ -397,7 +397,7 @@ static void make_file_index(void) debuga(_("Not enough memory to store the directory name \"%s\" in the index\n"),direntp->d_name); exit(EXIT_FAILURE); } - strncpy(item->date,data,sizeof(item->date)); + safe_strcpy(item->date,data,sizeof(item->date)); if (nsort+1>nallocated) { nallocated+=10; tempsort=realloc(sortlist,nallocated*sizeof(*item)); diff --git a/ip2name.c b/ip2name.c index 4657a51..5984005 100644 --- a/ip2name.c +++ b/ip2name.c @@ -67,8 +67,7 @@ void ip2name(char *ip,int ip_len) error=getnameinfo((struct sockaddr *)&sa,sizeof(sa),host,sizeof(host),NULL,0,0); } if (error==0) { - strncpy(ip,host,ip_len-1); - ip[ip_len-1]='\0'; + safe_strcpy(ip,host,ip_len); } else { debuga(_("IP to name resolution (getnameinfo) on IP address %s failed with error %d - %s\n"),ip,error,gai_strerror(error)); } @@ -93,8 +92,7 @@ void ip2name(char *ip,int ip_len) struct in_addr in; (void) memcpy(&in.s_addr, *p, sizeof (in.s_addr)); - strncpy(ip,hp->h_name,ip_len-1); - ip[ip_len-1]=0; + safe_strcpy(ip,hp->h_name,ip_len); } #endif return; diff --git a/log.c b/log.c index b1d14d2..8784dc6 100644 --- a/log.c +++ b/log.c @@ -378,27 +378,26 @@ int main(int argc,char *argv[]) lastlog=0; break; case 'a': - strcpy(addr,optarg); + safe_strcpy(addr,optarg,sizeof(addr)); break; case 'b': - strcpy(uagent,optarg); + safe_strcpy(uagent,optarg,sizeof(uagent)); break; case 'c': - strcpy(hexclude,optarg); + safe_strcpy(hexclude,optarg,sizeof(hexclude)); break; case 'd': - strncpy(date,optarg,sizeof(date)-1); - date[sizeof(date)-1]='\0'; + safe_strcpy(date,optarg,sizeof(date)); date_from(date, &dfrom, &duntil); break; case 'e': - strcpy(email,optarg); + safe_strcpy(email,optarg,sizeof(email)); break; case 'f': - strcpy(ConfigFile,optarg); + safe_strcpy(ConfigFile,optarg,sizeof(ConfigFile)); break; case 'g': - strcpy(df,optarg); + safe_strcpy(df,optarg,sizeof(df)); break; case 'h': usage(argv[0]); @@ -439,19 +438,19 @@ int main(int argc,char *argv[]) dns=true; break; case 'o': - strcpy(outdir,optarg); + safe_strcpy(outdir,optarg,sizeof(outdir)); break; case 'p': userip=true; break; case 'P': - strcpy(splitprefix,optarg); + safe_strcpy(splitprefix,optarg,sizeof(splitprefix)); break; case 'r': realt=true; break; case 's': - strcpy(site,optarg); + safe_strcpy(site,optarg,sizeof(site)); break; case 't': { @@ -477,13 +476,13 @@ int main(int argc,char *argv[]) break; } case 'u': - strcpy(us,optarg); + safe_strcpy(us,optarg,sizeof(us)); break; case 'v': version(); break; case 'w': - strcpy(tmp,optarg); + safe_strcpy(tmp,optarg,sizeof(tmp)); break; case 'x': debug++; @@ -1644,8 +1643,7 @@ int main(int argc,char *argv[]) char val4[255];//val4 must not be bigger than arq_log without fixing the strcpy below fclose(fp_log); - strncpy(end_hour,tbuf2,sizeof(end_hour)-1); - end_hour[sizeof(end_hour)-1]='\0'; + safe_strcpy(end_hour,tbuf2,sizeof(end_hour)); strftime(val2,sizeof(val2),"%d%m%Y",&period.start); strftime(val1,sizeof(val1),"%d%m%Y",&period.end); if (snprintf(val4,sizeof(val4),"%s/sarg-%s_%s-%s_%s.log",ParsedOutputLog,val2,start_hour,val1,end_hour)>=sizeof(val4)) { diff --git a/sort.c b/sort.c index fb67e76..56b6a0d 100644 --- a/sort.c +++ b/sort.c @@ -129,8 +129,7 @@ void sort_users_log(const char *tmp, int debug) if (dlen>0) { if (dlen>=sizeof(user)) continue; - strncpy(user,direntp->d_name,dlen); - user[dlen]=0; + safe_strcpy(user,direntp->d_name,dlen+1); } else { user[0]='\0'; } diff --git a/userinfo.c b/userinfo.c index 90c1433..4339ba6 100644 --- a/userinfo.c +++ b/userinfo.c @@ -89,8 +89,7 @@ struct userinfostruct *userinfo_create(const char *userid) } user=group->list+group->nusers++; - strncpy(user->id,userid,MAX_USER_LEN-1); - user->id[MAX_USER_LEN-1]='\0'; + safe_strcpy(user->id,userid,sizeof(user->id)); if (AnonymousOutputFiles) { snprintf(user->filename,sizeof(user->filename),"%d",AnonymousCounter++); diff --git a/usertab.c b/usertab.c index f338e7c..0cc83c8 100644 --- a/usertab.c +++ b/usertab.c @@ -116,18 +116,17 @@ static void get_usertab_name(const char *user,char *name,int namelen) char warea[MAXLEN]; char *str; - namelen--; sprintf(warea,"\t%s\n",user); if((str=(char *) strstr(userfile,warea)) == (char *) NULL ) { - strncpy(name,user,namelen); - name[namelen]=0; + safe_strcpy(name,user,namelen); } else { str=strchr(str+1,'\n'); str++; + namelen--; for(z1=0; *str != '\t' && z1text,sizeof(period->text),"%s-%s",text1,text2); snprintf(period->html,sizeof(period->html),"%s—%s",text1,text2); } else { - strncpy(period->text,text1,sizeof(period->text)-1); - period->text[sizeof(period->text)-1]='\0'; - strncpy(period->html,text1,sizeof(period->html)-1); - period->html[sizeof(period->html)-1]='\0'; + safe_strcpy(period->text,text1,sizeof(period->text)); + safe_strcpy(period->html,text1,sizeof(period->html)); } return(0); } @@ -1120,6 +1118,25 @@ int vrfydir(const struct periodstruct *per1, const char *addr, const char *site, return(0); } +/*! + Copy a string without overflowing the buffer. The copied string + is properly terminated by an ASCII zero. + + \param dest The destination buffer. + \param src The source buffer. + \param length The size of the destination buffer. The program is aborted + if the length is negative or zero. +*/ +void safe_strcpy(char *dest,const char *src,int length) +{ + if (length<=0) { + debuga(_("Invalid buffer length passed to the function to safely copy a string\n")); + exit(EXIT_FAILURE); + } + strncpy(dest,src,length-1); + dest[length-1]='\0'; +} + void strip_latin(char *line) { int i,j; -- 2.47.2