Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Jul 2013 14:53:32 +0200
From:      Ulrich =?utf-8?B?U3DDtnJsZWlu?= <uqs@FreeBSD.org>
To:        Pawel Jakub Dawidek <pjd@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r253457 - head/usr.bin/uniq
Message-ID:  <20130724125332.GC9092@acme.spoerlein.net>
In-Reply-To: <201307182211.r6IMBRYC091291@svn.freebsd.org>
References:  <201307182211.r6IMBRYC091291@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 2013-07-18 at 22:11:27 +0000, Pawel Jakub Dawidek wrote:
> Author: pjd
> Date: Thu Jul 18 22:11:27 2013
> New Revision: 253457
> URL: http://svnweb.freebsd.org/changeset/base/253457
> 
> Log:
>   Close uniq(1) in the capability mode sandbox and limit descriptors using
>   capability rights.
> 
> Modified:
>   head/usr.bin/uniq/uniq.c
> 
> Modified: head/usr.bin/uniq/uniq.c
> ==============================================================================
> --- head/usr.bin/uniq/uniq.c	Thu Jul 18 21:56:10 2013	(r253456)
> +++ head/usr.bin/uniq/uniq.c	Thu Jul 18 22:11:27 2013	(r253457)
> @@ -128,8 +145,34 @@ main (int argc, char *argv[])
>  	ofp = stdout;
>  	if (argc > 0 && strcmp(argv[0], "-") != 0)
>  		ifp = file(ifn = argv[0], "r");
> +	if (cap_rights_limit(fileno(ifp), CAP_FSTAT | CAP_READ) < 0 &&
> +	    errno != ENOSYS) {
> +		err(1, "unable to limit rights for %s", ifn);
> +	}
> +	rights = CAP_FSTAT | CAP_WRITE;
>  	if (argc > 1)
>  		ofp = file(argv[1], "w");
> +	else
> +		rights |= CAP_IOCTL;
> +	if (cap_rights_limit(fileno(ofp), rights) < 0 && errno != ENOSYS) {
> +		err(1, "unable to limit rights for %s",
> +		    argc > 1 ? argv[1] : "stdout");
> +	}
> +	if ((rights & CAP_IOCTL) != 0) {
> +		unsigned long cmd;
> +
> +		cmd = TIOCGETA; /* required by isatty(3) in printf(3) */
> +
> +		if (cap_ioctls_limit(fileno(ofp), &cmd, 1) < 0 &&
> +		    errno != ENOSYS) {
> +			err(1, "unable to limit ioctls for %s",
> +			    argc > 1 ? argv[1] : "stdout");
> +		}
> +	}

Deadcode, found by Coverity Scan, CID 1054780 (please mention in your
fix-commit). You check for argc > 1 at line 153, only if that is false
(meaning argc==1) do you set CAP_IOCTL. So on line 169 argc cannot be >1
and the result is always "stdout".

Cheers,
Uli



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130724125332.GC9092>