Tobias Stoeckmann wrote:
> 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.
ok mmcc@
It'd be nice to have a macro for specname's size, IMO. Using sizeof() in
strlcpy makes me uneasy and is less readable. That's a separate
improvement, though.
> 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.c25 Nov 2015 19:45:21 - 1.103
> +++ sbin/newfs/newfs.c5 Dec 2015 12:32:07 -
> @@ -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 - 1.21
> +++ sbin/newfs_ext2fs/newfs_ext2fs.c 5 Dec 2015 12:32:07 -
> @@ -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))
>
