Skip to content

Commit 6656f6f

Browse files
AtoptoolAtoptool
committed
Safer string copy (issue #335)
Function strncpy() replaced by internal function safe_strcpy() that always takes care of null-termination.
1 parent 45744c6 commit 6656f6f

10 files changed

+68
-40
lines changed

Diff for: ‎acctproc.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -973,8 +973,7 @@ acctphotoproc(struct tstat *accproc, int nrprocs)
973973
api->mem.majflt = acctexp(acctrec.ac_majflt);
974974
api->dsk.rio = acctexp(acctrec.ac_rw);
975975

976-
strncpy(api->gen.name, acctrec.ac_comm, PNAMLEN);
977-
api->gen.name[PNAMLEN] = '\0';
976+
safe_strcpy(api->gen.name, acctrec.ac_comm, sizeof api->gen.name);
978977
filled = 1;
979978
break;
980979

@@ -1002,8 +1001,7 @@ acctphotoproc(struct tstat *accproc, int nrprocs)
10021001
api->mem.majflt = acctexp(acctrec_v3.ac_majflt);
10031002
api->dsk.rio = acctexp(acctrec_v3.ac_rw);
10041003

1005-
strncpy(api->gen.name, acctrec_v3.ac_comm, PNAMLEN);
1006-
api->gen.name[PNAMLEN] = '\0';
1004+
safe_strcpy(api->gen.name, acctrec_v3.ac_comm, sizeof api->gen.name);
10071005
filled = 1;
10081006
break;
10091007
}

Diff for: ‎atop.c

+6-7
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ main(int argc, char *argv[])
379379
if (optind >= argc)
380380
prusage(argv[0]);
381381

382-
strncpy(orawname, argv[optind++], RAWNAMESZ-1);
382+
safe_strcpy(orawname, argv[optind++], sizeof orawname);
383383

384384
if (!rawwriteflag)
385385
{
@@ -402,8 +402,7 @@ main(int argc, char *argv[])
402402
}
403403
else
404404
{
405-
strncpy(irawname, argv[optind],
406-
RAWNAMESZ-1);
405+
safe_strcpy(irawname, argv[optind], sizeof irawname);
407406
optind++;
408407
}
409408
}
@@ -417,8 +416,8 @@ main(int argc, char *argv[])
417416
{
418417
if (*(argv[optind]) == '/')
419418
{
420-
strncpy(twindir, argv[optind],
421-
RAWNAMESZ-1);
419+
safe_strcpy(twindir, argv[optind],
420+
sizeof twindir);
422421
optind++;
423422
}
424423
}
@@ -1377,8 +1376,8 @@ twinprepare(void)
13771376
/*
13781377
** define current raw file name for both parent and child
13791378
*/
1380-
strncpy(irawname, tempname, RAWNAMESZ-1);
1381-
strncpy(orawname, tempname, RAWNAMESZ-1);
1379+
safe_strcpy(irawname, tempname, sizeof irawname);
1380+
safe_strcpy(orawname, tempname, sizeof orawname);
13821381
}
13831382

13841383
/*

Diff for: ‎atop.h

+2
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ int val2elapstr(int, char *);
179179
char *uid2name(uid_t);
180180
char *gid2name(gid_t);
181181

182+
void safe_strcpy(char *, const char *, size_t);
183+
182184
int compcpu(const void *, const void *);
183185
int compdsk(const void *, const void *);
184186
int compmem(const void *, const void *);

Diff for: ‎atophide.c

+26-9
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,19 @@ anonymize(struct sstat *ssp, struct tstat *tsp, int ntask)
361361
char *standin, *p;
362362

363363
// anonimize system-level stats
364+
//
365+
// be sure for that old value is fully wiped, especially
366+
// when the original string is langer that the substitute
367+
//
364368
// - logical volume names
365369
//
366370
for (i=0; i < ssp->dsk.nlvm; i++)
367371
{
368372
standin = findstandin(&lvmhead, &lvmsequence,
369373
"logvol", ssp->dsk.lvm[i].name);
370374

371-
memset(ssp->dsk.lvm[i].name, '\0', MAXDKNAM);
372-
strncpy(ssp->dsk.lvm[i].name, standin, MAXDKNAM-1);
375+
memset(ssp->dsk.lvm[i].name, '\0', sizeof ssp->dsk.lvm[i].name);
376+
safe_strcpy(ssp->dsk.lvm[i].name, standin, sizeof ssp->dsk.lvm[i].name);
373377
}
374378

375379
// anonimize system-level stats
@@ -382,8 +386,8 @@ anonymize(struct sstat *ssp, struct tstat *tsp, int ntask)
382386

383387
memset(ssp->nfs.nfsmounts.nfsmnt[i].mountdev, '\0',
384388
sizeof ssp->nfs.nfsmounts.nfsmnt[i].mountdev);
385-
strncpy(ssp->nfs.nfsmounts.nfsmnt[i].mountdev, standin,
386-
sizeof ssp->nfs.nfsmounts.nfsmnt[i].mountdev - 1);
389+
safe_strcpy(ssp->nfs.nfsmounts.nfsmnt[i].mountdev, standin,
390+
sizeof ssp->nfs.nfsmounts.nfsmnt[i].mountdev);
387391
}
388392

389393
// anonymize process-level stats
@@ -402,7 +406,7 @@ anonymize(struct sstat *ssp, struct tstat *tsp, int ntask)
402406
// remove command line arguments
403407
//
404408
if ( (p = strchr(tsp->gen.cmdline, ' ')) )
405-
memset(p, '\0', CMDLEN-(p-tsp->gen.cmdline));
409+
memset(p, '\0', CMDLEN-(p - tsp->gen.cmdline));
406410

407411
// check all allowed names
408412
//
@@ -422,11 +426,11 @@ anonymize(struct sstat *ssp, struct tstat *tsp, int ntask)
422426
standin = findstandin(&cmdhead, &cmdsequence,
423427
"prog", tsp->gen.name);
424428

425-
memset(tsp->gen.name, '\0', PNAMLEN+1);
426-
strncpy(tsp->gen.name, standin, PNAMLEN);
429+
memset(tsp->gen.name, '\0', sizeof tsp->gen.name);
430+
safe_strcpy(tsp->gen.name, standin, sizeof tsp->gen.name);
427431

428-
memset(tsp->gen.cmdline, '\0', CMDLEN+1);
429-
strncpy(tsp->gen.cmdline, standin, CMDLEN);
432+
memset(tsp->gen.cmdline, '\0', sizeof tsp->gen.cmdline);
433+
safe_strcpy(tsp->gen.cmdline, standin, sizeof tsp->gen.cmdline);
430434
}
431435
}
432436
}
@@ -849,3 +853,16 @@ getbranchtime(char *itim, time_t *newtime)
849853
*newtime = epoch;
850854
return 1;
851855
}
856+
857+
// copy a string to a destination buffer that will always
858+
// be null-terminated
859+
//
860+
void
861+
safe_strcpy(char *dst, const char *src, size_t dstsize)
862+
{
863+
if (dstsize == 0)
864+
return;
865+
866+
dst[0] = '\0';
867+
strncat(dst, src, dstsize - 1);
868+
}

Diff for: ‎atopsar.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,10 @@ atopsar(int argc, char *argv[])
173173
break;
174174

175175
case 'r': /* reading of file data ? */
176-
strncpy(irawname, optarg, RAWNAMESZ-1);
176+
safe_strcpy(irawname, optarg, RAWNAMESZ);
177177

178178
if (strcmp(irawname, "-") == 0)
179-
strcpy(irawname, "/dev/stdin");
179+
safe_strcpy(irawname, "/dev/stdin", RAWNAMESZ);
180180

181181
rawreadflag++;
182182
break;

Diff for: ‎gpucom.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -536,8 +536,8 @@ gpustat_parse(int version, char *buf, int maxgpu,
536536
if (! gpuparse(version, start, gg))
537537
return -1;
538538

539-
strncpy(gg->type, gputypes[gpunum], MAXGPUTYPE);
540-
strncpy(gg->busid, gpubusid[gpunum], MAXGPUBUS);
539+
safe_strcpy(gg->type, gputypes[gpunum], sizeof gg->type);
540+
safe_strcpy(gg->busid, gpubusid[gpunum], sizeof gg->busid);
541541

542542
/*
543543
** continue searching for per-process stats for this GPU

Diff for: ‎ifprop.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ getphysprop(struct ifprop *p)
331331
*/
332332
memset(&ifreq, 0, sizeof ifreq);
333333

334-
strncpy((void *)&ifreq.ifr_ifrn.ifrn_name, p->name,
335-
sizeof ifreq.ifr_ifrn.ifrn_name-1);
334+
safe_strcpy((void *)&ifreq.ifr_ifrn.ifrn_name, p->name,
335+
sizeof ifreq.ifr_ifrn.ifrn_name);
336336

337337
#ifdef ETHTOOL_GLINKSETTINGS
338338
ethlink = calloc(1, sizeof *ethlink);
@@ -417,8 +417,8 @@ getphysprop(struct ifprop *p)
417417
*/
418418
memset(&iwreq, 0, sizeof iwreq);
419419

420-
strncpy(iwreq.ifr_ifrn.ifrn_name, p->name,
421-
sizeof iwreq.ifr_ifrn.ifrn_name-1);
420+
safe_strcpy(iwreq.ifr_ifrn.ifrn_name, p->name,
421+
sizeof iwreq.ifr_ifrn.ifrn_name);
422422

423423
if ( ioctl(sockfd, SIOCGIWRATE, &iwreq) == 0)
424424
{

Diff for: ‎photosyst.c

+4-8
Original file line numberDiff line numberDiff line change
@@ -2102,8 +2102,7 @@ static void
21022102
nullmodname(unsigned int major, unsigned int minor,
21032103
char *curname, struct perdsk *px, int maxlen)
21042104
{
2105-
strncpy(px->name, curname, maxlen-1);
2106-
*(px->name+maxlen-1) = 0;
2105+
safe_strcpy(px->name, curname, maxlen);
21072106
}
21082107

21092108
static void
@@ -2178,8 +2177,7 @@ lvmmapname(unsigned int major, unsigned int minor,
21782177
/*
21792178
** store info in hash list
21802179
*/
2181-
strncpy(dmp->name, dentry->d_name, MAXDKNAM);
2182-
dmp->name[MAXDKNAM-1] = 0;
2180+
safe_strcpy(dmp->name, dentry->d_name, sizeof dmp->name);
21832181
dmp->major = major(statbuf.st_rdev);
21842182
dmp->minor = minor(statbuf.st_rdev);
21852183

@@ -2209,8 +2207,7 @@ lvmmapname(unsigned int major, unsigned int minor,
22092207
/*
22102208
** info found in hash list; fill proper name
22112209
*/
2212-
strncpy(px->name, dmp->name, maxlen-1);
2213-
*(px->name+maxlen-1) = 0;
2210+
safe_strcpy(px->name, dmp->name, maxlen);
22142211
return;
22152212
}
22162213

@@ -2220,8 +2217,7 @@ lvmmapname(unsigned int major, unsigned int minor,
22202217
/*
22212218
** info not found in hash list; fill original name
22222219
*/
2223-
strncpy(px->name, curname, maxlen-1);
2224-
*(px->name+maxlen-1) = 0;
2220+
safe_strcpy(px->name, curname, maxlen);
22252221
}
22262222

22272223
/*

Diff for: ‎showgeneric.c

+3-4
Original file line numberDiff line numberDiff line change
@@ -3790,8 +3790,7 @@ do_username(char *name, char *val)
37903790
{
37913791
struct passwd *pwd;
37923792

3793-
strncpy(procsel.username, val, sizeof procsel.username -1);
3794-
procsel.username[sizeof procsel.username -1] = 0;
3793+
safe_strcpy(procsel.username, val, sizeof procsel.username);
37953794

37963795
if (procsel.username[0])
37973796
{
@@ -3849,7 +3848,7 @@ do_username(char *name, char *val)
38493848
void
38503849
do_procname(char *name, char *val)
38513850
{
3852-
strncpy(procsel.progname, val, sizeof procsel.progname -1);
3851+
safe_strcpy(procsel.progname, val, sizeof procsel.progname);
38533852
procsel.prognamesz = strlen(procsel.progname);
38543853

38553854
if (procsel.prognamesz)
@@ -3994,7 +3993,7 @@ do_twindir(char *name, char *val)
39943993
{
39953994
extern char twindir[];
39963995

3997-
strncpy(twindir, val, RAWNAMESZ-1);
3996+
safe_strcpy(twindir, val, RAWNAMESZ);
39983997
}
39993998

40003999
void

Diff for: ‎various.c

+17
Original file line numberDiff line numberDiff line change
@@ -1170,3 +1170,20 @@ gid2name(gid_t gid)
11701170

11711171
return NULL;
11721172
}
1173+
1174+
1175+
/*
1176+
** copy a string to a destination buffer that will always
1177+
** be null-terminated
1178+
** 'dstsize' is supposed to be the total size of the
1179+
** destination buffer including null-byte
1180+
*/
1181+
void
1182+
safe_strcpy(char *dst, const char *src, size_t dstsize)
1183+
{
1184+
if (dstsize == 0)
1185+
return;
1186+
1187+
dst[0] = '\0';
1188+
strncat(dst, src, dstsize - 1);
1189+
}

0 commit comments

Comments
 (0)