Subject: bin/25762: strlcpy/strlcat could be utilized better
To: None <gnats-bugs@gnats.NetBSD.org>
From: Peter Postma <peter@pointless.nl>
List: netbsd-bugs
Date: 05/31/2004 15:34:42
>Number: 25762
>Category: bin
>Synopsis: strlcpy/strlcat could be utilized better
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: bin-bug-people
>State: open
>Class: change-request
>Submitter-Id: net
>Arrival-Date: Mon May 31 13:35:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator: Peter Postma
>Release: NetBSD 2.0E
>Organization:
>Environment:
System: NetBSD mercury.pointless.nl 2.0E NetBSD 2.0E (mercury) #9: Thu May 6 18:17:12 CEST 2004 peter@mercury.pointless.nl:/usr/obj/sys/arch/sparc64/compile/mercury sparc64
Architecture: sparc64
Machine: sparc64
>Description:
strlcpy and strlcat could be utilized better at various places.
Constructs like:
(void)strncpy(foo, bar, sizeof(foo) - 1);
foo[sizeof(foo) - 1] = '\0';
can be replaced with:
(void)strlcpy(foo, bar, sizeof(foo));
which is shorter and easier to read.
There are also some places where a NUL-termination is forced
after strlcpy/strlcat such as:
(void)strlcpy(savehost, krb_get_phost(hostname), sizeof(savehost));
savehost[sizeof(savehost) - 1] = '\0';
The NUL-termination here is redundant because strlcpy/strlcat always
NUL-terminate the destination. So that line can be removed.
>How-To-Repeat:
Inspect code.
>Fix:
I've checked bin, games, lib, libexec, usr.bin, usr.sbin and sbin
with a command for such appearances and fixed most of them.
Here's a diff:
Index: games/atc/log.c
===================================================================
RCS file: /cvsroot/src/games/atc/log.c,v
retrieving revision 1.12
diff -u -r1.12 log.c
--- games/atc/log.c 7 Aug 2003 09:36:54 -0000 1.12
+++ games/atc/log.c 31 May 2004 13:16:43 -0000
@@ -180,8 +180,7 @@
}
strcpy(thisscore.name, pw->pw_name);
uname(&name);
- strncpy(thisscore.host, name.nodename, sizeof(thisscore.host)-1);
- thisscore.host[sizeof(thisscore.host) - 1] = '\0';
+ strlcpy(thisscore.host, name.nodename, sizeof(thisscore.host));
cp = strrchr(file, '/');
if (cp == NULL) {
Index: games/sail/dr_1.c
===================================================================
RCS file: /cvsroot/src/games/sail/dr_1.c,v
retrieving revision 1.19
diff -u -r1.19 dr_1.c
--- games/sail/dr_1.c 7 Aug 2003 09:37:41 -0000 1.19
+++ games/sail/dr_1.c 31 May 2004 13:16:44 -0000
@@ -429,10 +429,8 @@
*tp = toupper(*tp);
p = tp;
}
- strncpy(bestship->file->captain, p,
+ strlcpy(bestship->file->captain, p,
sizeof bestship->file->captain);
- bestship->file->captain
- [sizeof bestship->file->captain - 1] = 0;
logger(bestship);
}
return -1;
Index: games/sail/sync.c
===================================================================
RCS file: /cvsroot/src/games/sail/sync.c,v
retrieving revision 1.22
diff -u -r1.22 sync.c
--- games/sail/sync.c 27 Jan 2004 20:27:59 -0000 1.22
+++ games/sail/sync.c 31 May 2004 13:16:44 -0000
@@ -379,9 +379,8 @@
break;
}
case W_CAPTAIN:
- strncpy(ship->file->captain, astr,
- sizeof ship->file->captain - 1);
- ship->file->captain[sizeof ship->file->captain - 1] = 0;
+ strlcpy(ship->file->captain, astr,
+ sizeof ship->file->captain);
break;
case W_CAPTURED:
if (a < 0)
@@ -418,9 +417,8 @@
ship->specs->hull = a;
break;
case W_MOVE:
- strncpy(ship->file->movebuf, astr,
- sizeof ship->file->movebuf - 1);
- ship->file->movebuf[sizeof ship->file->movebuf - 1] = 0;
+ strlcpy(ship->file->movebuf, astr,
+ sizeof ship->file->movebuf);
break;
case W_PCREW:
ship->file->pcrew = a;
Index: lib/libc/gen/sysctl.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/sysctl.c,v
retrieving revision 1.26
diff -u -r1.26 sysctl.c
--- lib/libc/gen/sysctl.c 25 Apr 2004 05:47:52 -0000 1.26
+++ lib/libc/gen/sysctl.c 31 May 2004 13:16:48 -0000
@@ -317,10 +317,9 @@
if (sysctl_usermib[ni].sysctl_desc == NULL)
d1->descr_len = 1;
else {
- strncpy(d1->descr_str,
+ (void)strlcpy(d1->descr_str,
sysctl_usermib[ni].sysctl_desc,
sizeof(buf) - sizeof(*d1));
- buf[sizeof(buf) - 1] = '\0';
d1->descr_len = strlen(d1->descr_str) + 1;
}
d = (size_t)__sysc_desc_adv(NULL, d1->descr_len);
Index: lib/libpcap/inet.c
===================================================================
RCS file: /cvsroot/src/lib/libpcap/inet.c,v
retrieving revision 1.8
diff -u -r1.8 inet.c
--- lib/libpcap/inet.c 13 Apr 2000 05:14:19 -0000 1.8
+++ lib/libpcap/inet.c 31 May 2004 13:16:51 -0000
@@ -137,8 +137,7 @@
return (NULL);
}
- (void)strncpy(device, mp->ifa_name, sizeof(device) - 1);
- device[sizeof(device) - 1] = '\0';
+ (void)strlcpy(device, mp->ifa_name, sizeof(device));
freeifaddrs(ifap);
return (device);
#else
@@ -220,8 +219,7 @@
return (NULL);
}
- (void)strncpy(device, mp->ifr_name, sizeof(device) - 1);
- device[sizeof(device) - 1] = '\0';
+ (void)strlcpy(device, mp->ifr_name, sizeof(device));
return (device);
#endif
}
Index: lib/libwrap/diag.c
===================================================================
RCS file: /cvsroot/src/lib/libwrap/diag.c,v
retrieving revision 1.7
diff -u -r1.7 diag.c
--- lib/libwrap/diag.c 24 Sep 2001 17:55:47 -0000 1.7
+++ lib/libwrap/diag.c 31 May 2004 13:16:51 -0000
@@ -75,8 +75,7 @@
/* append format and force null termination */
fmt[o] = '\0';
- strncat(fmt, format, sizeof(fmt) - o);
- fmt[sizeof(fmt) - 1] = '\0';
+ (void)strlcat(fmt, format, sizeof(fmt) - o);
errno = oerrno;
vsyslog(severity, fmt, ap);
Index: libexec/rpc.rquotad/rquotad.c
===================================================================
RCS file: /cvsroot/src/libexec/rpc.rquotad/rquotad.c,v
retrieving revision 1.21
diff -u -r1.21 rquotad.c
--- libexec/rpc.rquotad/rquotad.c 20 Sep 2003 22:25:29 -0000 1.21
+++ libexec/rpc.rquotad/rquotad.c 31 May 2004 13:16:52 -0000
@@ -376,8 +376,7 @@
*uqfnamep = NULL;
*gqfnamep = NULL;
- strncpy(buf, fs->fs_mntops, sizeof(buf) - 1);
- buf[sizeof(buf) - 1] = '\0';
+ (void)strlcpy(buf, fs->fs_mntops, sizeof(buf));
for (opt = strtok(buf, ","); opt; opt = strtok(NULL, ",")) {
if ((cp = strchr(opt, '=')))
*cp++ = '\0';
Index: usr.bin/quota/quota.c
===================================================================
RCS file: /cvsroot/src/usr.bin/quota/quota.c,v
retrieving revision 1.29
diff -u -r1.29 quota.c
--- usr.bin/quota/quota.c 21 Apr 2004 01:05:47 -0000 1.29
+++ usr.bin/quota/quota.c 31 May 2004 13:16:53 -0000
@@ -534,8 +534,7 @@
qfextension[GRPQUOTA], qfname);
initname = 1;
}
- (void)strncpy(buf, fs->fs_mntops, sizeof(buf) - 1);
- buf[sizeof(buf) - 1] = '\0';
+ (void)strlcpy(buf, fs->fs_mntops, sizeof(buf));
for (opt = strtok(buf, ","); opt; opt = strtok(NULL, ",")) {
if ((cp = strchr(opt, '=')) != NULL)
*cp++ = '\0';
Index: usr.bin/su/su.c
===================================================================
RCS file: /cvsroot/src/usr.bin/su/su.c,v
retrieving revision 1.58
diff -u -r1.58 su.c
--- usr.bin/su/su.c 5 Jan 2004 23:23:37 -0000 1.58
+++ usr.bin/su/su.c 31 May 2004 13:16:54 -0000
@@ -622,7 +622,6 @@
hostname[sizeof(hostname) - 1] = '\0';
(void)strlcpy(savehost, krb_get_phost(hostname), sizeof(savehost));
- savehost[sizeof(savehost) - 1] = '\0';
kerno = krb_mk_req(&ticket, "rcmd", savehost, lrealm, 33);
Index: usr.bin/tip/tip.c
===================================================================
RCS file: /cvsroot/src/usr.bin/tip/tip.c,v
retrieving revision 1.26
diff -u -r1.26 tip.c
--- usr.bin/tip/tip.c 23 Apr 2004 22:24:34 -0000 1.26
+++ usr.bin/tip/tip.c 31 May 2004 13:16:54 -0000
@@ -133,8 +133,7 @@
(int)sizeof(PNbuf) - 1);
exit(1);
}
- strncpy(PNbuf, System, sizeof PNbuf - 1);
- PNbuf[sizeof PNbuf - 1] = '\0';
+ (void)strlcpy(PNbuf, System, sizeof(PNbuf));
for (p = System; *p; p++)
*p = '\0';
PN = PNbuf;
Index: usr.bin/who/utmpentry.c
===================================================================
RCS file: /cvsroot/src/usr.bin/who/utmpentry.c,v
retrieving revision 1.4
diff -u -r1.4 utmpentry.c
--- usr.bin/who/utmpentry.c 12 Feb 2003 17:39:36 -0000 1.4
+++ usr.bin/who/utmpentry.c 31 May 2004 13:16:55 -0000
@@ -250,12 +250,9 @@
static void
getentry(struct utmpentry *e, struct utmp *up)
{
- (void)strncpy(e->name, up->ut_name, sizeof(up->ut_name));
- e->name[sizeof(e->name) - 1] = '\0';
- (void)strncpy(e->line, up->ut_line, sizeof(up->ut_line));
- e->line[sizeof(e->line) - 1] = '\0';
- (void)strncpy(e->host, up->ut_host, sizeof(up->ut_host));
- e->name[sizeof(e->host) - 1] = '\0';
+ (void)strlcpy(e->name, up->ut_name, sizeof(up->ut_name));
+ (void)strlcpy(e->line, up->ut_line, sizeof(up->ut_line));
+ (void)strlcpy(e->host, up->ut_host, sizeof(up->ut_host));
e->tv.tv_sec = up->ut_time;
e->tv.tv_usec = 0;
adjust_size(e);
@@ -266,12 +263,9 @@
static void
getentryx(struct utmpentry *e, struct utmpx *up)
{
- (void)strncpy(e->name, up->ut_name, sizeof(up->ut_name));
- e->name[sizeof(e->name) - 1] = '\0';
- (void)strncpy(e->line, up->ut_line, sizeof(up->ut_line));
- e->line[sizeof(e->line) - 1] = '\0';
- (void)strncpy(e->host, up->ut_host, sizeof(up->ut_host));
- e->name[sizeof(e->host) - 1] = '\0';
+ (void)strlcpy(e->name, up->ut_name, sizeof(up->ut_name));
+ (void)strlcpy(e->line, up->ut_line, sizeof(up->ut_line));
+ (void)strlcpy(e->host, up->ut_host, sizeof(up->ut_host));
e->tv = up->ut_tv;
adjust_size(e);
}
Index: usr.sbin/bootp/bootpd/bootpd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/bootp/bootpd/bootpd.c,v
retrieving revision 1.19
diff -u -r1.19 bootpd.c
--- usr.sbin/bootp/bootpd/bootpd.c 14 Jul 2003 06:08:04 -0000 1.19
+++ usr.sbin/bootp/bootpd/bootpd.c 31 May 2004 13:16:56 -0000
@@ -878,10 +878,8 @@
if (bootfile) {
if (bootfile[0] != '/') {
strlcat(realpath, "/", sizeof(realpath));
- realpath[sizeof(realpath) - 1] = '\0';
}
strlcat(realpath, bootfile, sizeof(realpath));
- realpath[sizeof(realpath) - 1] = '\0';
bootfile = NULL;
}
Index: usr.sbin/rarpd/rarpd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/rarpd/rarpd.c,v
retrieving revision 1.51
diff -u -r1.51 rarpd.c
--- usr.sbin/rarpd/rarpd.c 12 May 2004 16:48:44 -0000 1.51
+++ usr.sbin/rarpd/rarpd.c 31 May 2004 13:16:58 -0000
@@ -337,8 +337,7 @@
if (ioctl(fd, BIOCSBLEN, &bufsize) < 0) {
rarperr(NONFATAL, "BIOCSBLEN:%d: %s", bufsize, strerror(errno));
}
- (void)strncpy(ifr.ifr_name, device, sizeof ifr.ifr_name - 1);
- ifr.ifr_name[sizeof ifr.ifr_name - 1] = '\0';
+ (void)strlcpy(ifr.ifr_name, device, sizeof(ifr.ifr_name));
if (ioctl(fd, BIOCSETIF, (caddr_t) & ifr) < 0) {
if (aflag) { /* for -a skip non-ethernet interfaces */
close(fd);
Index: usr.sbin/rpc.bootparamd/bootparamd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/rpc.bootparamd/bootparamd.c,v
retrieving revision 1.42
diff -u -r1.42 bootparamd.c
--- usr.sbin/rpc.bootparamd/bootparamd.c 25 Dec 2003 19:01:35 -0000 1.42
+++ usr.sbin/rpc.bootparamd/bootparamd.c 31 May 2004 13:16:58 -0000
@@ -180,13 +180,10 @@
(char *) &whoami->client_address.bp_address_u.ip_addr,
sizeof(haddr));
he = gethostbyaddr((char *) &haddr, sizeof(haddr), AF_INET);
- if (he) {
- strncpy(askname, he->h_name, sizeof(askname));
- askname[sizeof(askname)-1] = 0;
- } else {
- strncpy(askname, inet_ntoa(haddr), sizeof(askname));
- askname[sizeof(askname)-1] = 0;
- }
+ if (he)
+ (void)strlcpy(askname, he->h_name, sizeof(askname));
+ else
+ (void)strlcpy(askname, inet_ntoa(haddr), sizeof(askname));
if (debug)
warnx("This is host %s", askname);
@@ -259,8 +256,7 @@
return (NULL);
}
- strncpy(askname, he->h_name, sizeof(askname));
- askname[sizeof(askname)-1] = 0;
+ (void)strlcpy(askname, he->h_name, sizeof(askname));
err = lookup_bootparam(askname, NULL, getfile->file_id,
&res.server_name, &res.server_path);
if (err == 0) {
Index: usr.sbin/tpctl/tp.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/tpctl/tp.c,v
retrieving revision 1.2
diff -u -r1.2 tp.c
--- usr.sbin/tpctl/tp.c 3 Jan 2003 04:41:49 -0000 1.2
+++ usr.sbin/tpctl/tp.c 31 May 2004 13:16:59 -0000
@@ -75,8 +75,7 @@
id.type = WSMOUSE_ID_TYPE_UIDSTR;
if (ioctl(tp->fd, WSMOUSEIO_GETID, &id) == 0) {
- strncpy(tp->id, id.data, MIN(sizeof(tp->id), id.length));
- tp->id[sizeof(tp->id) - 1] = '\0';
+ (void)strlcpy(tp->id, id.data, MIN(sizeof(tp->id), id.length));
} else {
tp->id[0] = '*';
tp->id[1] = '\0';
Index: usr.sbin/ypbind/ypbind.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/ypbind/ypbind.c,v
retrieving revision 1.51
diff -u -r1.51 ypbind.c
--- usr.sbin/ypbind/ypbind.c 5 Jan 2004 23:23:39 -0000 1.51
+++ usr.sbin/ypbind/ypbind.c 31 May 2004 13:17:00 -0000
@@ -186,8 +186,7 @@
}
(void)memset(ypdb, 0, sizeof *ypdb);
- (void)strncpy(ypdb->dom_domain, dm, sizeof ypdb->dom_domain);
- ypdb->dom_domain[sizeof(ypdb->dom_domain) - 1] = '\0';
+ (void)strlcpy(ypdb->dom_domain, dm, sizeof ypdb->dom_domain);
return ypdb;
}
Index: usr.sbin/ypserv/ypserv/ypserv_db.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/ypserv/ypserv/ypserv_db.c,v
retrieving revision 1.15
diff -u -r1.15 ypserv_db.c
--- usr.sbin/ypserv/ypserv/ypserv_db.c 16 Jul 2003 06:57:39 -0000 1.15
+++ usr.sbin/ypserv/ypserv/ypserv_db.c 31 May 2004 13:17:00 -0000
@@ -513,8 +513,7 @@
if (host == NULL)
return (YP_NOKEY);
- strncpy((char *) hostname, host->h_name, sizeof(hostname) - 1);
- hostname[sizeof(hostname) - 1] = '\0';
+ (void)strlcpy(hostname, host->h_name, sizeof(hostname));
host = gethostbyname(hostname);
if (host == NULL)
return (YP_NOKEY);
Index: usr.sbin/ypset/ypset.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/ypset/ypset.c,v
retrieving revision 1.15
diff -u -r1.15 ypset.c
--- usr.sbin/ypset/ypset.c 10 Dec 2003 12:06:26 -0000 1.15
+++ usr.sbin/ypset/ypset.c 31 May 2004 13:17:00 -0000
@@ -130,8 +130,7 @@
gethostaddr(server, &ypsd.ypsetdom_addr);
- (void) strncpy(ypsd.ypsetdom_domain, dom, sizeof ypsd.ypsetdom_domain);
- ypsd.ypsetdom_domain[sizeof(ypsd.ypsetdom_domain) - 1] = '\0';
+ (void) strlcpy(ypsd.ypsetdom_domain, dom, sizeof ypsd.ypsetdom_domain);
ypsd.ypsetdom_port = port;
ypsd.ypsetdom_vers = YPVERS;
Index: sbin/modload/modload.c
===================================================================
RCS file: /cvsroot/src/sbin/modload/modload.c,v
retrieving revision 1.45
diff -u -r1.45 modload.c
--- sbin/modload/modload.c 19 Mar 2004 12:04:37 -0000 1.45
+++ sbin/modload/modload.c 31 May 2004 13:17:01 -0000
@@ -322,8 +322,7 @@
err(3, _PATH_LKM);
fileopen |= DEV_OPEN;
- strncpy(modout, modobj, sizeof(modout) - 1);
- modout[sizeof(modout) - 1] = '\0';
+ (void)strlcpy(modout, modobj, sizeof(modout));
p = strrchr(modout, '.');
if (!p || strcmp(p, ".o"))
>Release-Note:
>Audit-Trail:
>Unformatted: