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))
