Thread: setuid(geteuid());?
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
> 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
thespecified 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,
Pennsylvania19026
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
>> 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
> 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, Pennsylvania19026
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
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
> 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, Pennsylvania19026
> 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,
Pennsylvania19026
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
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
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
> > 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, Pennsylvania19026
> 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, Pennsylvania19026
> 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
thespecified 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
functionsreturn -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,
Pennsylvania19026
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
theeuid 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
memberof 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
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
> 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, Pennsylvania19026