From owner-freebsd-audit  Sun Aug 19 12: 7:27 2001
Delivered-To: freebsd-audit@freebsd.org
Received: from coffee.q9media.com (coffee.q9media.com [216.94.229.19])
	by hub.freebsd.org (Postfix) with ESMTP id 8D0DE37B40D
	for <audit@FreeBSD.org>; Sun, 19 Aug 2001 12:07:23 -0700 (PDT)
	(envelope-from mike@coffee.q9media.com)
Received: (from mike@localhost)
	by coffee.q9media.com (8.11.2/8.11.3) id f7JJTRp49176;
	Sun, 19 Aug 2001 15:29:27 -0400 (EDT)
	(envelope-from mike)
Date: Sun, 19 Aug 2001 15:29:27 -0400
From: Mike Barcroft <mike@FreeBSD.org>
To: Kris Kennaway <kris@obsecurity.org>
Cc: audit@FreeBSD.org
Subject: Re: Checking issetugid() with getenv() in libraries
Message-ID: <20010819152927.A49129@coffee.q9media.com>
Mail-Followup-To: Mike Barcroft <mike@FreeBSD.org>,
	Kris Kennaway <kris@obsecurity.org>, audit@FreeBSD.org
References: <20010818221258.A79194@xor.obsecurity.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <20010818221258.A79194@xor.obsecurity.org>; from kris@obsecurity.org on Sat, Aug 18, 2001 at 10:12:58PM -0700
Organization: The FreeBSD Project
Sender: owner-freebsd-audit@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-audit.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-audit>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-audit>
X-Loop: FreeBSD.ORG

Kris Kennaway <kris@obsecurity.org> writes:
> There were a number of places where library routines blindly use
> getenv() in ways which may be insecure if called from setugid code.
> Please review the following.
> 
> I also changed the uthread_info.c to respect TMPDIR if !issetugid()
> instead of dumping to /tmp always.
> 
> Kris
> 
> Index: libc/db/test/dbtest.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libc/db/test/dbtest.c,v
> retrieving revision 1.4
> diff -u -r1.4 dbtest.c
> --- libc/db/test/dbtest.c	2000/08/04 10:50:21	1.4
> +++ libc/db/test/dbtest.c	2001/08/19 04:25:47
> @@ -155,7 +155,8 @@
>  	 * want it around, and it often screws up tests.
>  	 */
>  	if (fname == NULL) {
> -		p = getenv("TMPDIR");
> +		if (issetugid() == 0)
> +			p = getenv("TMPDIR");
>  		if (p == NULL)
>  			p = "/var/tmp";
>  		(void)snprintf(buf, sizeof(buf), "%s/__dbtest", p);
[...]

Is p initialized to NULL above this?  If not, p could be uninitialized
when issetugid() != 0.

> Index: libc/stdio/tmpfile.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libc/stdio/tmpfile.c,v
> retrieving revision 1.6
> diff -u -r1.6 tmpfile.c
> --- libc/stdio/tmpfile.c	2001/07/07 04:08:32	1.6
> +++ libc/stdio/tmpfile.c	2001/08/19 04:19:53
> @@ -61,7 +61,8 @@
>  	char *buf;
>  	const char *tmpdir;
>  
> -	tmpdir = getenv("TMPDIR");
> +	if (issetugid() == 0)
> +		tmpdir = getenv("TMPDIR");
>  	if (tmpdir == NULL)
>  		tmpdir = _PATH_TMP;
>  
[...]

There's a similar problem here.

> Index: libncp/ncpl_rcfile.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libncp/ncpl_rcfile.c,v
> retrieving revision 1.3
> diff -u -r1.3 ncpl_rcfile.c
> --- libncp/ncpl_rcfile.c	2000/05/26 02:00:20	1.3
> +++ libncp/ncpl_rcfile.c	2001/08/19 04:52:39
> @@ -390,7 +390,8 @@
>  	char *home, *fn;
>  	int error;
>  
> -	home = getenv("HOME");
> +	if (issetugid() == 0)
> +		home = getenv("HOME");
>  	if (home) {
>  		fn = malloc(strlen(home) + 20);
>  		sprintf(fn, "%s/.nwfsrc", home);
[...]

Also here.

The rest of the changes look good.

Best regards,
Mike Barcroft

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message