Tom Lane writes:

> Bruce Momjian <[EMAIL PROTECTED]> 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   [EMAIL PROTECTED]   http://funkturm.homeip.net/~peter


---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Reply via email to