This call

setuid(geteuid());

is found in backend/utils/init/postinit.c.  AFAICT, this does nothing.
Anyone got an idea?

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Sat, 21 Apr 2001 17:04:34 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
setuid(geteuid());?
> This call
> 
> setuid(geteuid());
> 
> is found in backend/utils/init/postinit.c.  AFAICT, this does nothing.
> Anyone got an idea?

Well, from my BSD manual it says:

     The setuid() function sets the real and effective user IDs and the saved
     set-user-ID of the current process to the specified value.  The setuid()

so it seems to make sure the real/saved uid matches the effective uid. 
Now, considering we don't use uid/euid distinction for anything, I agree
it is useless and should be removed.

I don't see any mention of getuid() except in odbc and pg_id.  Seems
they should be geteuid() too.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 11:07:56 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> so it seems to make sure the real/saved uid matches the effective uid. 
> Now, considering we don't use uid/euid distinction for anything, I agree
> it is useless and should be removed.

No, it is NOT useless and must NOT be removed.  The point of this little
machination is to be dead certain that we have given up root rights if
executed as setuid postgres.  The scenario we're concerned about is
where real uid = root and effective uid = postgres.  We want real uid
to become postgres as well --- otherwise our test to prevent execution
as root is a waste of time, because nefarious code could become root
again just by doing setuid.  See the setuid man page: if real uid is
root then setuid(root) will succeed.

			regards, tom lane

Sat, 21 Apr 2001 12:29:47 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
>> so it seems to make sure the real/saved uid matches the effective uid. 
>> Now, considering we don't use uid/euid distinction for anything, I agree
>> it is useless and should be removed.

> No, it is NOT useless and must NOT be removed.

But it would make sense to move it over to main.c where the primary
test for not running as root is, since these are closely related
considerations: we don't want either euid or ruid to be root.

I'll make that change unless I hear objections...

			regards, tom lane

Sat, 21 Apr 2001 12:43:37 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > so it seems to make sure the real/saved uid matches the effective uid. 
> > Now, considering we don't use uid/euid distinction for anything, I agree
> > it is useless and should be removed.
> 
> No, it is NOT useless and must NOT be removed.  The point of this little
> machination is to be dead certain that we have given up root rights if
> executed as setuid postgres.  The scenario we're concerned about is
> where real uid = root and effective uid = postgres.  We want real uid
> to become postgres as well --- otherwise our test to prevent execution
> as root is a waste of time, because nefarious code could become root
> again just by doing setuid.  See the setuid man page: if real uid is
> root then setuid(root) will succeed.

I understand, but how do we get suid execution.  Does someone have to
set the seuid bit on the executable?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 12:51:45 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I understand, but how do we get suid execution.  Does someone have to
> set the seuid bit on the executable?

Probably so, but I could see someone thinking they could do that as a
substitute for saying "su - postgres" on every startup.

If we are going to take the trouble to refuse to run when euid = 0,
then it also behooves us to guard against ruid = 0.

			regards, tom lane

Sat, 21 Apr 2001 12:58:30 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I understand, but how do we get suid execution.  Does someone have to
> > set the seuid bit on the executable?
> 
> Probably so, but I could see someone thinking they could do that as a
> substitute for saying "su - postgres" on every startup.
> 
> If we are going to take the trouble to refuse to run when euid = 0,
> then it also behooves us to guard against ruid = 0.

OK, that's what I thought.  The command is not needed in our default
configuration.  I agree we should prevent people from setting up bad
configurations if we can.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 13:08:53 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
> That is a valid concern, but the code doesn't actually prevent this.  I
> just tried
> 
> chmod u+s postgres
> su -
> postmaster -D ...
> 
> Then loaded the function
> 
> #include <postgres.h>
> 
> int32 touch(int32 a) {
>     if (setuid(0) == -1)
>         elog(ERROR, "setuid: %m");
>     elog(DEBUG, "getuid = %d, geteuid = %d", getuid(), geteuid());
>     system("touch /tmp/foofile");
>     setuid(500); /* my own */
>     return a + 1;
> }
> 
> and the output was
> 
> DEBUG:  getuid = 0, geteuid = 0
> 
> and I got a file /tmp/foofile owned by root.
> 
> ISTM that the best way to prevent this exploit would be to check for both
> geteuid() == 0 and getuid() == 0 in main.c.

Peter, can you check your setuid manual page.  Is there a mention of
special handling of saved-uid for root?  I don't have it here on BSD/OS
but have heard of some os's that treat setuid differently for root.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 13:10:33 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
Tom Lane writes:

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > so it seems to make sure the real/saved uid matches the effective uid.
> > Now, considering we don't use uid/euid distinction for anything, I agree
> > it is useless and should be removed.
>
> No, it is NOT useless and must NOT be removed.  The point of this little
> machination is to be dead certain that we have given up root rights if
> executed as setuid postgres.  The scenario we're concerned about is
> where real uid = root and effective uid = postgres.

If effective uid = postgres, then this will execute setuid(postgres),
which does nothing.

> We want real uid
> to become postgres as well --- otherwise our test to prevent execution
> as root is a waste of time, because nefarious code could become root
> again just by doing setuid.  See the setuid man page: if real uid is
> root then setuid(root) will succeed.

That is a valid concern, but the code doesn't actually prevent this.  I
just tried

chmod u+s postgres
su -
postmaster -D ...

Then loaded the function

#include <postgres.h>

int32 touch(int32 a) {
    if (setuid(0) == -1)
        elog(ERROR, "setuid: %m");
    elog(DEBUG, "getuid = %d, geteuid = %d", getuid(), geteuid());
    system("touch /tmp/foofile");
    setuid(500); /* my own */
    return a + 1;
}

and the output was

DEBUG:  getuid = 0, geteuid = 0

and I got a file /tmp/foofile owned by root.

ISTM that the best way to prevent this exploit would be to check for both
geteuid() == 0 and getuid() == 0 in main.c.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Sat, 21 Apr 2001 19:17:34 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: setuid(geteuid());? 
Peter Eisentraut <peter_e@gmx.net> writes:
>> We want real uid
>> to become postgres as well --- otherwise our test to prevent execution
>> as root is a waste of time, because nefarious code could become root
>> again just by doing setuid.  See the setuid man page: if real uid is
>> root then setuid(root) will succeed.

> That is a valid concern, but the code doesn't actually prevent this.

After reading the setuid man page a third time, I think you are right.

On machines that have setreuid(), or even better setresuid(), we could
force the ruid (and suid for good measure) to match euid.  Otherwise we
probably should refuse to start unless getuid matches geteuid.

Hmm ... setresuid may be an HP-ism ... does anyone else have that?
setreuid appears to be a BSD-ism, so it ought to be reasonably popular.

			regards, tom lane

Sat, 21 Apr 2001 13:29:13 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
I said:
> On machines that have setreuid(), or even better setresuid(), we could
> force the ruid (and suid for good measure) to match euid.  Otherwise we
> probably should refuse to start unless getuid matches geteuid.

But on third thought, it's not worth the trouble of adding two more
configure tests to support a configuration that I doubt anyone uses
anyway (ie, setuid postgres executable).  Let's just remove the setuid()
and add a check for getuid() == geteuid() in main.c.

Peter, unless you've already started in on this, I can take care of it
--- I see a couple of other nits I want to fix in those two files, too.

			regards, tom lane

Sat, 21 Apr 2001 13:47:23 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
Tom Lane writes:

> I said:
> > On machines that have setreuid(), or even better setresuid(), we could
> > force the ruid (and suid for good measure) to match euid.  Otherwise we
> > probably should refuse to start unless getuid matches geteuid.
>
> But on third thought, it's not worth the trouble of adding two more
> configure tests to support a configuration that I doubt anyone uses
> anyway (ie, setuid postgres executable).  Let's just remove the setuid()
> and add a check for getuid() == geteuid() in main.c.
>
> Peter, unless you've already started in on this, I can take care of it
> --- I see a couple of other nits I want to fix in those two files, too.

Please do.  I just ran across it for the session authorization deal, which
would be easy to merge.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Sat, 21 Apr 2001 20:13:57 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: setuid(geteuid());? 
> > That is a valid concern, but the code doesn't actually prevent this.
> 
> After reading the setuid man page a third time, I think you are right.
> 
> On machines that have setreuid(), or even better setresuid(), we could
> force the ruid (and suid for good measure) to match euid.  Otherwise we
> probably should refuse to start unless getuid matches geteuid.
> 
> Hmm ... setresuid may be an HP-ism ... does anyone else have that?
> setreuid appears to be a BSD-ism, so it ought to be reasonably popular.

I have setreuid on BSD/OS, no setresuid.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 15:57:49 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
> I said:
> > On machines that have setreuid(), or even better setresuid(), we could
> > force the ruid (and suid for good measure) to match euid.  Otherwise we
> > probably should refuse to start unless getuid matches geteuid.
> 
> But on third thought, it's not worth the trouble of adding two more
> configure tests to support a configuration that I doubt anyone uses
> anyway (ie, setuid postgres executable).  Let's just remove the setuid()
> and add a check for getuid() == geteuid() in main.c.
> 
> Peter, unless you've already started in on this, I can take care of it
> --- I see a couple of other nits I want to fix in those two files, too.

Good. That function call clearly needs a comment added.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 15:58:36 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
> Tom Lane writes:
> 
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > so it seems to make sure the real/saved uid matches the effective uid.
> > > Now, considering we don't use uid/euid distinction for anything, I agree
> > > it is useless and should be removed.
> >
> > No, it is NOT useless and must NOT be removed.  The point of this little
> > machination is to be dead certain that we have given up root rights if
> > executed as setuid postgres.  The scenario we're concerned about is
> > where real uid = root and effective uid = postgres.
> 
> If effective uid = postgres, then this will execute setuid(postgres),
> which does nothing.

I am a little confused.  BSD/OS manual page says:

     The setuid() function sets the real and effective user IDs and the saved
     set-user-ID of the current process to the specified value.  The setuid()
     function is permitted if the specified ID is equal to the real user ID of
     the process, or if the effective user ID is that of the super user.

...

     If the user is not the super user, or the uid specified is not the real,
     effective ID, or saved ID, these functions return -1.

so why does your test work?  Does your manual say something different?
If setuid() sets user/effective/saved to postgres, how can you get back
root?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 16:03:10 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?
Peter Eisentraut <peter_e@gmx.net> writes:
> Bruce Momjian writes:
>> so why does your test work?  Does your manual say something different?
>> If setuid() sets user/effective/saved to postgres, how can you get back
>> root?

> : setuid  sets  the  effective user ID of the current process.  If the
> : effective userid of the caller is root, the real and saved user ID's
> : are also set.

HPUX has an even more bizarre definition:

     setuid() sets the real-user-ID (ruid),effective-user-ID (euid), and/or
     saved-user-ID (suid) of the calling process.  The super-user's euid is
     zero.  The following conditions govern setuid's behavior:

          o  If the euid is zero, setuid() sets the ruid, euid, and suid to
             uid.

          o  If the euid is not zero, but the argument uid is equal to the
             ruid or the suid, setuid() sets the euid to uid; the ruid and
             suid remain unchanged.  (If a set-user-ID program is not
             running as super-user, it can change its euid to match its
             ruid and reset itself to the previous euid value.)

          o  If euid is not zero, but the argument uid is equal to the
             euid, and the calling process is a member of a group that has
             the PRIV_SETRUGID privilege (see privgrp(4)), setuid() sets
             the ruid to uid; the euid and suid remain unchanged.

Rule #2 is what creates the security hole.  Rule #3 would allow us to
plug the hole, but only if we have PRIV_SETRUGID...

			regards, tom lane

Sat, 21 Apr 2001 16:42:03 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: setuid(geteuid());? 
Bruce Momjian writes:

> so why does your test work?  Does your manual say something different?
> If setuid() sets user/effective/saved to postgres, how can you get back
> root?

: setuid  sets  the  effective user ID of the current process.  If the
: effective userid of the caller is root, the real and saved user ID's
: are also set.
:
: Under  Linux,  setuid is implemented like the POSIX version with the
: _POSIX_SAVED_IDS feature.  This allows a setuid  (other  than  root)
: program  to  drop  all of its user privileges, do some un-privileged
: work, and then re-engage the original effective user ID in a  secure
: manner.

I suppose your system doesn't have the _POSIX_SAVED_IDS feature.

I also have:

: CONFORMING TO
:        SVr4, SVID, POSIX.1.  Not quite compatible  with  the  4.4BSD  call,
:        which  sets  all  of  the real, saved, and effective user IDs.

On your system you would have to use seteuid() to do what setuid() does
here.

One more reason to avoid this area when possible.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Sat, 21 Apr 2001 22:43:26 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: setuid(geteuid());?
> HPUX has an even more bizarre definition:
> 
>      setuid() sets the real-user-ID (ruid),effective-user-ID (euid), and/or
>      saved-user-ID (suid) of the calling process.  The super-user's euid is
>      zero.  The following conditions govern setuid's behavior:
> 
>           o  If the euid is zero, setuid() sets the ruid, euid, and suid to
>              uid.
> 
>           o  If the euid is not zero, but the argument uid is equal to the
>              ruid or the suid, setuid() sets the euid to uid; the ruid and
>              suid remain unchanged.  (If a set-user-ID program is not
>              running as super-user, it can change its euid to match its
>              ruid and reset itself to the previous euid value.)
> 
>           o  If euid is not zero, but the argument uid is equal to the
>              euid, and the calling process is a member of a group that has
>              the PRIV_SETRUGID privilege (see privgrp(4)), setuid() sets
>              the ruid to uid; the euid and suid remain unchanged.
> 
> Rule #2 is what creates the security hole.  Rule #3 would allow us to
> plug the hole, but only if we have PRIV_SETRUGID...

I don't even want to twist my brain far enough to understand this.  So
basically. BSD/OS is safe with a seteuid executable if we keep the
setuid(geteuid()) call, while other OS's have serious problems we can't
plug.  I knew there was some OS-specific stuff in setuid.  Seems a check
that uid and euid are the same and not root is the way to go.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 21 Apr 2001 17:53:31 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: setuid(geteuid());?