Subject: Re: Addition to force open to open only regular files
To: NetBSD Kernel Technical Discussion List <>
From: Robert Elz <kre@munnari.OZ.AU>
List: tech-kern
Date: 11/25/2000 13:34:34
    Date:        Fri, 24 Nov 2000 02:14:55 -0500 (EST)
    From: (Greg A. Woods)
    Message-ID:  <>

  | In the end it all depends on just how eager you are to avoid what I
  | suspect is the primary cause of buffer overflow and printf format trust
  | exploits in set-ID programs -- i.e. exploits that are made possible by
  | saved-set-ID swapping.

Do you have any evidence of this at all?    It sounds totally bogus
to me.   It sounds as if you're confusing "code run as the user"
with "the user's code" though surely it cannot be that.

Without doubt, all code in a setuid binary (or at least, all code before
it does setuid(getuid())) needs to be very carefully sanitised.  That
includes code that is run after an ID swap.   Buffer overflows (and such)
are no more allowable there than they are in any other part of a setuid
program, and I can't imagine why anyone would believe that a programmer
who was insanely careful to always do ...

	fgets(buf, sizeof buf, stdin);
	strncpy(buf, from, sizeof buf);

and such is suddenly going to start writing

	strcpy(buf, from);

just because there's a seteuid(getuid()) call preceding the latter code.

That simply doesn't make sense.   Nor should be be assuming that the
library routines that setuid programs might be calling are full of
such nonsense, and hence automatically vunerable to attack - but if we
are to assume that, why would we be assuming that those library calls
will be made only after seteuid(getuid()) and not before it?

I have now been totally convinced that open_as() is a total waste of
time, and offers nothing at all.   Given that that you have said
that the only uid/gid pair that you would ever use there are getuid()
and getgid(), which means actually passing them through the syscall is
just a waste of time - so better would be realopen() with the same args
as open, but using different credentials to do the permission checks.
Then of course, to be complete, we also need realunlink() reallink()
realrename(), realmkdir(), realrmdir(), realstat() reallstat(), realsymlink()
realmkfifo(), realsocket() (for the unix domain case) .....

Rather than add a dozen (or more) new syscalls, then it starts looking
a lot simpler to simply add a syscall to say "apply the real ids to the
next syscall I make", so you would do something like
	fd = open();
which would have exactly the same effect as your open_as() the way you
claim it must be used.  But be much more general.

But then, it seems like the sequence

	euid = geteuid();
	fd = open();

has exactly the same effects as the above - aside from the paranoid
desire to make the last of those illegal.   ie: nothing new is needed
at all.

In that precise usage, I can't see how even you can complain that
there's any difference at all between the above sequence and the
open_as() you're championing.

Of course, using the "revert to privileged" paradigm it is possible to
write brain dead code that is insecure.   But it is possible to write
brain dead code that is insecure without any kind of id-swapping at all.
	main() { execl("/bin/sh", "sh", (char *)0); }
compile that, make it setuid, and security is gone - and not a single
use of reverting to previous uids, or anything like it involved.

Most likely the uid swapping and swap back adds some potential problems
that weren't there before - but so does everything new - nothing comes
without some risk.   Again, what is important is to evaluate the risk,
see if it is possible to be safe, if so, document that method, and then
leave it to people to do things the right way.

There seem to be some involved in this debate who want to make it easy
for people to write setuid programs that are safe - that's an insane
desire, there's no point even attempting it.   Writing setuid programs
has always been hard, and will always be hard.