Subject: Re: Addition to force open to open only regular files
To: NetBSD Kernel Technical Discussion List <tech-kern@NetBSD.ORG>
From: Greg A. Woods <woods@weird.com>
List: tech-kern
Date: 11/24/2000 01:47:13
[ On Thursday, November 23, 2000 at 18:19:03 (-0500), Matthew Orgass wrote: ]
> Subject: Re: Addition to force open to open only regular files
>
>   This is not true.  In fact, setre[ug]id offers two features not
> available otherwise: 1) root-started programs can pretend they were
> started by another user and/or group,

This has always been possible, from day one, with setuid(getuid()).

> and 2) non-root programs can setuid
> to the effective user id.

What, exactly, is that supposed to mean?

>  #1 is valid,

Of course!

> but #2 should have been done by
> simply allowing setuid to the effective user id (so they are never
> swapped) and not allowing non-root users to access setreuid.

setuid(geteuid()) is in effect a no-OP [unless some other set*id() call
has already intervened], except that it doesn't affect the real-IDs.

In any case please note that setreuid() has long been deprecated, and
not just because of this issue -- it should never be used by any current
set-ID programs in the first place, and so far as I'm aware it is no
longer used in any of the base NetBSD source.

>   Both switching to the real id and open_as do not really fix the problem
> because the id of the user who execed the program may no longer be
> available at all, as in the case of the user program that effectively does
> setuid(geteuid()).

I'm not sure which list my message explaining why this does not matter
was posted to, but please understand that this does not matter.  If a
set-ID program calls setuid(geteuid()) [or otherwise passes the
effective-UID to setuid()] then this can only be because the program
has, as you say pretend, to take on the effective-ID in such a way that
it is impossible to know that it was not initially started by the user
to whom the set-ID program is owned.

In all cases the result of getuid() [i.e. the real-UID], is always
sufficient to be used with open_as(), regardless of whether or not the
process was set-ID, or not, and regardless of whether or not the process
has first called setuid().  In either case the real-ID will always be
the correct one to use as the credentials for the file open, based on
the self-obvious intent of the caller.

>  Furthermore, checking for setuid does not fix the
> problem because the program could then exec another program, keeping the
> same environment, that then uses the tainted envrionment variable (this
> kills file-id).

Exactly, which is why saved-set-ID swapping must be completely disabled,
since doing so will make it easier for a set-ID program to permanently
avoid passing a potentially dangeous environment on to some unsuspecting
non-setuid program.  This is, of course, because it will be easier for
set-ID programs to decide to permanently drop their privileges as soon
as they possibly can.

>  Disallowing library functions from accessing devices does
> not work when the kernel does not know that a file refers to a device (as
> could be the case on a network file system).

Well, actually, any network file system which permits opens of devices
must clearly identify the fact that the file is a device to the client
kernel.  If this is not so then the remote filesystem in question is
extremely buggy and untrustworthy in the first place.  Note that neither
NFS nor RFS suffer from such bugs.  If you have a real example of such
bugs, then please share it with us.

>  Also, opening a regular file
> may have side effects in non-traditional file systems, such as a portal
> file or a file system that can fetch files automaticly from off-line
> media.  The abstraction needed to determine if it is safe to open a file
> does not exist.

Hmmm....  yes, indeed.  So far I've avoided raising this argument
because with open_as() it is irrelevant -- permissions are sufficient to
do the protections and thus avoid the side effects.  However it is
indeed another example of how the current implementations of
$HOSTALIASES and $TZ are dangerous in general.

>   The real problem here is not considering the environment to be tainted.

Yes of course -- this is one of the top ten rules for set-ID programmers
to be aware of.  The problem here is not that this is a new issue, but
rather that several extremely popular standard library routines have
made use of environment variable values as filenames and such routines
are almost impossible for at least some classes of set-ID programs to
avoid.  Since such set-ID programs do not explicitly sanitise their
environment, and since doing so would anger at least some users, another
solution is cried out for.

Either such library routines must be severely constrained, or some way
must be found to make them safe.  If there were no way to exploit race
conditions then checking access() before calling open() would be
sufficient.  Clearly this is not the case though so something without a
race condition must be used.

> One way to make this clear is to start setuid programs with an empty
> environment and an additional tainted environment that contains the
> environment passed to execve.

And some people thought what I was proposing would be controversial!
You've just thrown in the towel, I think!  :-)

>   The question now becomes how to untaint the environment for execed
> programs and how to safely use as much of the passed environment as is
> wanted in this program.  This may mean load the TZ now with the real ID
> but do not set the environment or access that file later.  Many setuid
> programs might never want to accept a user supplied environment variable
> and doing so may have security implications even if the actual reading of
> that variable is done securely.

This is already a rule that set-ID programmers *should* be extremely
aware of.  However they are not always so diligent in practice.

Unfortunately the forced hand-holding you suggest is not going to be
acceptable to many people.

I believe though that removing saved-set-ID swapping and in its place
offering a truly safe way for even a library routine to open any
arbitrary file with just the real-ID credentials will mitigate the
potential dangers such that an exploit attempt can be discovered before
it can be taken advantage of.

>   It will always be possible to abuse setuid, and the current proposals
> make the problem worse instead of better.  The current setuid model is
> fine, even setreuid; what needs to be done is to make it possible to
> untaint the environment in various conditions and document how to do so.

Of course set-ID abuses will always be possible!  Good luck with trying
to base your solution solely on sanitizing the environment though!

You're assuming the crackers will fight fair and that set-ID programmers
will always follow all of the rules.  Neither assumption is true and
there are dozens, if not hundreds, of examples of real exploits that
have already happened because such assumtions have been broken, and
there's a certainty that there will be more if your proposal is
followed.  I've all but argued that 100% of buffer overflow, printf
format trust violations, and other similar types of exploits against
set-ID programs are due explicitly to the fact that saved-set-ID
swapping is permitted.  I still won't go quite so far as to claim this,
but as you can no doubt tell I'm reasonably sure that saved-set-ID
swapping is a major culprit in these matters!

In the real world there's a proven need for systems programmers to
mitigate the possible damages that can be done by set-ID programs which
do not follow all of the de facto rules explicitly.  As I've shown this
is possible by removing saved-set-ID swapping.  Doing so however
requires that an open_as() call be added to avoid the access()/open()
race condition since it would otherwise eliminate the most commonly used
solution to this proglem.

The only thing I worry about is that open_as() in the form I've
specified it is not safe enough.  Perhaps it should be restricted to
always use only the real-IDs as the credentials to check, which would
imply that the saved-set-ID credentials must be completely removed from
the kernel too.  That would of course make the name "open_as()"
misleading -- perhaps such an interface would better be called just
"open_as_real()"?  This added restriction would also make fixing current
set-ID programs somewhat more difficult, but still not really all that
hard to do in the long run....

-- 
							Greg A. Woods

+1 416 218-0098      VE3TCP      <gwoods@acm.org>      <robohack!woods>
Planix, Inc. <woods@planix.com>; Secrets of the Weird <woods@weird.com>