svn commit: r186291 - head/sbin/mount

[M. Warner Losh]

In message: <200812181844.mBIIikVF049455 at svn.freebsd.org>
            "David E. O'Brien" <obrien at FreeBSD.org> writes:
: Author: obrien
: Date: Thu Dec 18 18:44:46 2008
: New Revision: 186291
: URL: http://svn.freebsd.org/changeset/base/186291
: 
: Log:
:   Be a little bit more pestimistic in argument handling - check if we've
:   overflown our internal buffer (though after the fact), and s/strncpy/strlcpy/
:   
:   Reviewed by:	rodrigc
:   Obtained from:	Juniper Networks
: 
: Modified:
:   head/sbin/mount/mount.c
:   head/sbin/mount/mount_fs.c
: 
: Modified: head/sbin/mount/mount.c
: ==============================================================================
: --- head/sbin/mount/mount.c	Thu Dec 18 18:29:15 2008	(r186290)
: +++ head/sbin/mount/mount.c	Thu Dec 18 18:44:46 2008	(r186291)
: @@ -68,6 +68,8 @@ static const char rcsid[] =
:  #define MOUNT_META_OPTION_FSTAB		"fstab"
:  #define MOUNT_META_OPTION_CURRENT	"current"
:  
: +#define	MAX_ARGS			100
: +
:  int debug, fstab_style, verbose;
:  
:  char   *catopt(char *, const char *);
: @@ -501,7 +503,7 @@ int
:  mountfs(const char *vfstype, const char *spec, const char *name, int flags,
:  	const char *options, const char *mntopts)
:  {
: -	char *argv[100];
: +	char *argv[MAX_ARGS];
:  	struct statfs sf;
:  	int argc, i, ret;
:  	char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX];
: @@ -546,6 +548,10 @@ mountfs(const char *vfstype, const char 
:  	argv[argc++] = strdup(name);
:  	argv[argc] = NULL;
:  
: +	if (MAX_ARGS <= argc )
: +		errx(1, "Cannot process more than %d mount arguments",
: +		    MAX_ARGS);
: +

This is useless.  Once you've overflowed the buffer, your stack is
potentially shot, and all kinds of fun can happen downstream from
there.  In particular, there's no guarantee that argc isn't corrupted
as well...

Also, the style of the check is inconsistent with other parts of the
system:

: +	if (argc > MAX_ARGS)

would be more consistent, and not suffer from the space before the
paren problem as well :)

Warner

:  	if (debug) {
:  		if (use_mountprog(vfstype))
:  			printf("exec: mount_%s", vfstype);
: 
: Modified: head/sbin/mount/mount_fs.c
: ==============================================================================
: --- head/sbin/mount/mount_fs.c	Thu Dec 18 18:29:15 2008	(r186290)
: +++ head/sbin/mount/mount_fs.c	Thu Dec 18 18:44:46 2008	(r186291)
: @@ -88,7 +88,7 @@ mount_fs(const char *vfstype, int argc, 
:  	char *p, *val;
:  	int ret;
:  
: -	strncpy(fstype, vfstype, sizeof(fstype));
: +	strlcpy(fstype, vfstype, sizeof(fstype));
:  	memset(errmsg, 0, sizeof(errmsg));
:  
:  	getmnt_silent = 1;
: