On Sat, Dec 05, 2015 at 06:26:35AM -0500, Ted Unangst wrote: > may i suggest strlen(s) instead of strchr(s, 0)?
There's actually one part in newfs' code that uses this. And in theory it has the same issue, not checking if s (which is special, which might be argv[0]) is empty. I highly doubt this could be reached there, but I fixed it anyway. Until now it uses strncpy, and with the switch to strlcpy this is just another additional boundary check in place. Tobias Index: sbin/newfs/newfs.c =================================================================== RCS file: /cvs/src/sbin/newfs/newfs.c,v retrieving revision 1.103 diff -u -p -u -p -r1.103 newfs.c --- sbin/newfs/newfs.c 25 Nov 2015 19:45:21 -0000 1.103 +++ sbin/newfs/newfs.c 5 Dec 2015 12:32:07 -0000 @@ -423,10 +423,11 @@ main(int argc, char *argv[]) warnx("%s: not a character-special device", special); } - cp = strchr(argv[0], '\0') - 1; - if (cp == NULL || - ((*cp < 'a' || *cp > ('a' + maxpartitions - 1)) - && !isdigit((unsigned char)*cp))) + if (*argv[0] == '\0') + fatal("empty partition name supplied"); + cp = argv[0] + strlen(argv[0]) - 1; + if ((*cp < 'a' || *cp > ('a' + maxpartitions - 1)) + && !isdigit((unsigned char)*cp)) fatal("%s: can't figure out file system partition", argv[0]); lp = getdisklabel(special, fsi); @@ -631,8 +632,9 @@ rewritelabel(char *s, int fd, struct dis /* * Make name for 'c' partition. */ - strncpy(specname, s, sizeof(specname) - 1); - specname[sizeof(specname) - 1] = '\0'; + if (*s == '\0' || + strlcpy(specname, s, sizeof(specname)) >= sizeof(specname)) + fatal("%s: invalid partition name supplied", s); cp = specname + strlen(specname) - 1; if (!isdigit((unsigned char)*cp)) *cp = 'c'; Index: sbin/newfs_ext2fs/newfs_ext2fs.c =================================================================== RCS file: /cvs/src/sbin/newfs_ext2fs/newfs_ext2fs.c,v retrieving revision 1.21 diff -u -p -u -p -r1.21 newfs_ext2fs.c --- sbin/newfs_ext2fs/newfs_ext2fs.c 28 Nov 2015 06:12:09 -0000 1.21 +++ sbin/newfs_ext2fs/newfs_ext2fs.c 5 Dec 2015 12:32:07 -0000 @@ -529,9 +529,11 @@ getpartition(int fsi, const char *specia errx(EXIT_FAILURE, "%s: block device", special); if (!S_ISCHR(st.st_mode)) warnx("%s: not a character-special device", special); - cp = strchr(argv[0], '\0') - 1; - if (cp == NULL || ((*cp < 'a' || *cp > ('a' + getmaxpartitions() - 1)) - && !isdigit((unsigned char)*cp))) + if (*argv[0] == '\0') + errx(EXIT_FAILURE, "empty partition name supplied"); + cp = argv[0] + strlen(argv[0]) - 1; + if ((*cp < 'a' || *cp > ('a' + getmaxpartitions() - 1)) + && !isdigit((unsigned char)*cp)) errx(EXIT_FAILURE, "%s: can't figure out file system partition", argv[0]); lp = getdisklabel(special, fsi); if (isdigit((unsigned char)*cp))