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/17/2000 12:54:30
[ On Friday, November 17, 2000 at 16:58:18 (+0700), Robert Elz wrote: ]
> Subject: Re: Addition to force open to open only regular files 
>
> Are you aware what you're really saying here?

I think so.

> HOSTALIASES isn't anything special, it is just an instance of the
> problem.  Making HOSTALIASES go away won't solve anything at all.
> (LOCALDOMAIN is a slightly different issue, as it doesn't name a file).

I realise that.

(LOCALDOMAIN affects the naming of hosts, which in this day and age is
almost as important as the naming of a file.)

> The more general problem is whether library routines (any that might ever be 
> used by any setuid program) can ever use any environment variables,
> and in particular, any which refer to files.

Yes, of course.  One of the top ten rules of writing set-ID programs is
to take extreme care when using environment variables, perhaps even more
extreme care than with any other user-supplied input data because of
this very fact.  "Caveat emptor" and all that -- if the programmer uses
a library routine and doesn't check to see if it accesses environment
variables, or uses them in unsafe ways, then the resulting set-ID
program is not going to be safe.

Of course if the documentation doesn't properly list all environment
variables used by a library routine, or doesn't correctly describe their
affects on its behaviour, then the OS "vendor" is partly to blame.

> TZ is another obvious example (and if you think there no are setuid
> programs around that blindly call ctime() ...)

Well, it doesn't refer to a file, and it doesn't change the way
anything's accessed, i.e. it doesn't affect name-to-location mapping, so
I don't think it falls into the same category.  Obviously code that uses
even such inocuous variables should be regularly audited to ensure that
it does so safely even in the face of out-of-spec values.

> I suspect there are a bunch more.

There very well could be.  Has anyone done a grep for getenv() in the
library tree recently?

> What is needed is a safe way to allow libraries to reference files
> that were passed in by name by the originating user - that, or setuid
> programs really simply need to be much more fastidious about what they
> do, and make sure that they know what the library routines they call
> might be coerced into doing, and avoid that.

In this particular case there's a bit of a trick in that the feature in
question is optional and non-obvious to the set-ID programmer.  Set-ID
programmers MUST always take extreme care in what library routines they
are linking into their code (either directly or indirectly).

Indeed set-ID programs already take care to access user-specified files
in as safe a manner as possible.  However it would make certain set-ID
programming tasks easier if there were an open_as() call, or if there
were a set of "filesystem-ID" credentials that could be set for all such
calls.  Introducing the latter will take some serious auditing though
since it will affect far more library routines than just the resolver.

> Just "disable HOSTALIASES" is the idiot's answer to the problem, it
> isn't a real answer at all.

No, it's not idiotic at all -- it's simply the only fix that's possible
given the circumstances.  You obviously can't disable all uses of the
resolver library by any set-ID program, so you're left with disabling
the *optional*, dangerous, feature.

Though of course even I have stated that in a risk analysis the danger
need not be that great if the code at least takes some precautions to
fstat() the file it's opened and scream loudly about security violations
if it finds it's opened a device file or a file not owned by either the
superuser or the real user.

What's left to discuss in this particular example is whether or not
HOSTALIASES (and perhaps LOCALDOMAIN) should only be disabled for all
set-ID programs (perhaps by checking if (getuid() != geteuid()) in the
library?), or mabye whether or not all support needs to be removed, and
if it is kept whether or not to add checks such as I've described above.
(That discussion should probably happen in a new thread though.)

If this discussion hadn't been started by a few people who wanted an old
and perhaps rarely used feature to continue to work, but instead had
started in a security audit of new code, there would be no question at
all about what to do with it.  It would seem to me that pride,
prejudice, and laziness are all that's driving the current attempt to
hold onto it despite its obvious and demonstrated danger.

(BTW, I'd like to have getsuid() and getsgid() calls to return the
saved-set-ID values.)

-- 
							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>