From owner-cvs-all  Sat Nov 25  2:12:16 2000
Delivered-To: cvs-all@freebsd.org
Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20])
	by hub.freebsd.org (Postfix) with ESMTP
	id B669637B4C5; Sat, 25 Nov 2000 02:12:11 -0800 (PST)
Received: (from bright@localhost)
	by fw.wintelcom.net (8.10.0/8.10.0) id eAPACBm15765;
	Sat, 25 Nov 2000 02:12:11 -0800 (PST)
Date: Sat, 25 Nov 2000 02:12:11 -0800
From: Alfred Perlstein <bright@wintelcom.net>
To: Brian Feldman <green@FreeBSD.org>
Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject: Re: cvs commit: src/usr.sbin/inetd builtins.c
Message-ID: <20001125021211.H8051@fw.wintelcom.net>
References: <200011250413.UAA16251@freefall.freebsd.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <200011250413.UAA16251@freefall.freebsd.org>; from green@FreeBSD.org on Fri, Nov 24, 2000 at 08:13:05PM -0800
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

* Brian Feldman <green@FreeBSD.org> [001124 20:13] wrote:
> green       2000/11/24 20:13:05 PST
> 
>   Modified files:
>     usr.sbin/inetd       builtins.c 
>   Log:
>   Security fix: correctly set groups according to the user.  Previously,
>   root's groups' permissions were being used, so a user could read up to
>   16 (excluding initial whitespace) bytes of e.g. a wheel-accessible file.

Was this reviewed by anyone?  Security fixes need review.

>   Also, don't allow blocking on the opening of ~/.fakeid, so replace a fopen()
>   with open() and fdopen().  I knew I'd be going to hell for using C file
>   streams instead of POSIX syscalls...

What happens if ~/.fakeid is a FIFO without a writer/reader?  Is
this the blocking you're trying to avoid?

I think you may have a problem here because you don't use fclose
later, you may be leaking stdio streams with no backing fd.

Don't you need to change:

+               if (fakeid_fd != -1)
+                       close(fakeid_fd);

to something like:

if (fakeid != NULL)
  fclose(fakeid);
else if fakeid_fd != -1)
  close(fakeid_fd);

There's also a problem because this close isn't part of the 'else'
clause above, what happens is that you may call close on fakeid_fd
(after the 'else' clause) after calling fclose on it already in
the 'if' clause.

Doesn't the fstat need to happen before the fdopen? (to avoid
blocking on FIFOs or other wierdness).

What's going on here?  And why was it MFC'd already?

Maybe I'm wrong but these are some _possible_ bugs that I discovered
after a cursory examination of the change, had it been dicussed
I'd have either realized I was wrong or we could have had a proper
fix the first time.

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."


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