Skip to content

Commit

Permalink
Remove gete?[ug]id caching
Browse files Browse the repository at this point in the history 
Currently we cache the UID/GID and effective UID/GID similarly to how
we used to cache getpid() before v5.14.0-251-g0e21945. Remove this
magical behavior in favor of always calling getpid(), getgid()
etc. This resolves RT #96208.

A minimal testcase for this is the following by Leon Timmermans
attached to RT #96208:

    eval { require 'syscall.ph'; 1 } or eval { require 'sys/syscall.ph'; 1 } or die $@;

    if (syscall(&SYS_setuid, $ARGV[0] + 0 || 1000) >= 0 or die "$!") {
            printf "\$< = %d, getuid = %d\n", $<, syscall(&SYS_getuid);
    }

I.e. if we call the sete?[ug]id() functions unbeknownst to perl the
$<, $>, $( and $) variables won't be updated. This results in the same
sort of issues we had with $$ before v5.14.0-251-g0e21945, and
getppid() before my v5.15.7-407-gd7c042c patch.

I'm completely eliminating the PL_egid, PL_euid, PL_gid and PL_uid
variables as part of this patch, this will break some CPAN modules,
but it'll be really easy before the v5.16.0 final to reinstate
them. I'd like to remove them to see what breaks, and how easy it is
to fix it.

These variables are not part of the public API, and the modules using
them could either use the Perl_gete?[ug]id() functions or are working
around the bug I'm fixing with this commit.

The new PL_delaymagic_(egid|euid|gid|uid) variables I'm adding are
*only* intended to be used internally in the interpreter to facilitate
the delaymagic in Perl_pp_sassign. There's probably some way not to
export these to programs that embed perl, but I haven't found out how
to do that.
  • Loading branch information
Ævar Arnfjörð Bjarmason committed Feb 18, 2012
1 parent f0bcc49 commit 985213f
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 115 deletions.
8 changes: 4 additions & 4 deletions doio.c
View file
Expand Up @@ -1771,7 +1771,7 @@ nothing in the core.
while (++mark <= sp) {
s = SvPV_nolen_const(*mark);
APPLY_TAINT_PROPER();
if (PL_euid || PL_unsafe) {
if (PerlProc_geteuid() || PL_unsafe) {
if (UNLINK(s))
tot--;
}
Expand Down Expand Up @@ -1909,7 +1909,7 @@ Perl_cando(pTHX_ Mode_t mode, bool effective, register const Stat_t *statbufp)
# ifdef __CYGWIN__
if (ingroup(544,effective)) { /* member of Administrators */
# else
if ((effective ? PL_euid : PL_uid) == 0) { /* root is special */
if ((effective ? PerlProc_geteuid() : PerlProc_getuid()) == 0) { /* root is special */
# endif
if (mode == S_IXUSR) {
if (statbufp->st_mode & 0111 || S_ISDIR(statbufp->st_mode))
Expand All @@ -1919,7 +1919,7 @@ Perl_cando(pTHX_ Mode_t mode, bool effective, register const Stat_t *statbufp)
return TRUE; /* root reads and writes anything */
return FALSE;
}
if (statbufp->st_uid == (effective ? PL_euid : PL_uid) ) {
if (statbufp->st_uid == (effective ? PerlProc_geteuid() : PerlProc_getuid()) ) {
if (statbufp->st_mode & mode)
return TRUE; /* ok as "user" */
}
Expand All @@ -1938,7 +1938,7 @@ static bool
S_ingroup(pTHX_ Gid_t testgid, bool effective)
{
dVAR;
if (testgid == (effective ? PL_egid : PL_gid))
if (testgid == (effective ? PerlProc_getegid() : PerlProc_getgid()))
return TRUE;
#ifdef HAS_GETGROUPS
{
Expand Down
8 changes: 4 additions & 4 deletions embedvar.h
View file
Expand Up @@ -149,6 +149,10 @@
#define PL_defoutgv (vTHX->Idefoutgv)
#define PL_defstash (vTHX->Idefstash)
#define PL_delaymagic (vTHX->Idelaymagic)
#define PL_delaymagic_egid (vTHX->Idelaymagic_egid)
#define PL_delaymagic_euid (vTHX->Idelaymagic_euid)
#define PL_delaymagic_gid (vTHX->Idelaymagic_gid)
#define PL_delaymagic_uid (vTHX->Idelaymagic_uid)
#define PL_destroyhook (vTHX->Idestroyhook)
#define PL_diehook (vTHX->Idiehook)
#define PL_doswitches (vTHX->Idoswitches)
Expand All @@ -158,13 +162,11 @@
#define PL_e_script (vTHX->Ie_script)
#define PL_efloatbuf (vTHX->Iefloatbuf)
#define PL_efloatsize (vTHX->Iefloatsize)
#define PL_egid (vTHX->Iegid)
#define PL_encoding (vTHX->Iencoding)
#define PL_endav (vTHX->Iendav)
#define PL_envgv (vTHX->Ienvgv)
#define PL_errgv (vTHX->Ierrgv)
#define PL_errors (vTHX->Ierrors)
#define PL_euid (vTHX->Ieuid)
#define PL_eval_root (vTHX->Ieval_root)
#define PL_eval_start (vTHX->Ieval_start)
#define PL_evalseq (vTHX->Ievalseq)
Expand All @@ -179,7 +181,6 @@
#define PL_formtarget (vTHX->Iformtarget)
#define PL_generation (vTHX->Igeneration)
#define PL_gensym (vTHX->Igensym)
#define PL_gid (vTHX->Igid)
#define PL_glob_index (vTHX->Iglob_index)
#define PL_globalstash (vTHX->Iglobalstash)
#define PL_globhook (vTHX->Iglobhook)
Expand Down Expand Up @@ -349,7 +350,6 @@
#define PL_tmps_stack (vTHX->Itmps_stack)
#define PL_top_env (vTHX->Itop_env)
#define PL_toptarget (vTHX->Itoptarget)
#define PL_uid (vTHX->Iuid)
#define PL_unicode (vTHX->Iunicode)
#define PL_unitcheckav (vTHX->Iunitcheckav)
#define PL_unitcheckav_save (vTHX->Iunitcheckav_save)
Expand Down
14 changes: 0 additions & 14 deletions ext/POSIX/POSIX.xs
View file
Expand Up @@ -1907,24 +1907,10 @@ sleep(seconds)
SysRet
setgid(gid)
Gid_t gid
CLEANUP:
#ifndef WIN32
if (RETVAL >= 0) {
PL_gid = getgid();
PL_egid = getegid();
}
#endif

SysRet
setuid(uid)
Uid_t uid
CLEANUP:
#ifndef WIN32
if (RETVAL >= 0) {
PL_uid = getuid();
PL_euid = geteuid();
}
#endif

SysRetLong
sysconf(name)
Expand Down
8 changes: 4 additions & 4 deletions intrpvar.h
View file
Expand Up @@ -464,10 +464,10 @@ PERLVARI(I, in_clean_all, bool, FALSE) /* ptrs to freed SVs now legal */
PERLVAR(I, nomemok, bool) /* let malloc context handle nomem */
PERLVARI(I, savebegin, bool, FALSE) /* save BEGINs for compiler */

PERLVAR(I, uid, Uid_t) /* current real user id */
PERLVAR(I, euid, Uid_t) /* current effective user id */
PERLVAR(I, gid, Gid_t) /* current real group id */
PERLVAR(I, egid, Gid_t) /* current effective group id */
PERLVAR(I, delaymagic_uid, Uid_t) /* current real user id, only for delaymagic */
PERLVAR(I, delaymagic_euid, Uid_t) /* current effective user id, only for delaymagic */
PERLVAR(I, delaymagic_gid, Gid_t) /* current real group id, only for delaymagic */
PERLVAR(I, delaymagic_egid, Gid_t) /* current effective group id, only for delaymagic */
PERLVARI(I, an, U32, 0) /* malloc sequence number */

#ifdef DEBUGGING
Expand Down
81 changes: 43 additions & 38 deletions mg.c
View file
Expand Up @@ -1109,16 +1109,16 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
SvNOK_on(sv); /* what a wonderful hack! */
break;
case '<':
sv_setiv(sv, (IV)PL_uid);
sv_setiv(sv, (IV)PerlProc_getuid());
break;
case '>':
sv_setiv(sv, (IV)PL_euid);
sv_setiv(sv, (IV)PerlProc_geteuid());
break;
case '(':
sv_setiv(sv, (IV)PL_gid);
sv_setiv(sv, (IV)PerlProc_getgid());
goto add_groups;
case ')':
sv_setiv(sv, (IV)PL_egid);
sv_setiv(sv, (IV)PerlProc_getegid());
add_groups:
#ifdef HAS_GETGROUPS
{
Expand Down Expand Up @@ -2795,89 +2795,94 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)
}
break;
case '<':
PL_uid = SvIV(sv);
{
const IV new_uid = SvIV(sv);
PL_delaymagic_uid = new_uid;
if (PL_delaymagic) {
PL_delaymagic |= DM_RUID;
break; /* don't do magic till later */
}
#ifdef HAS_SETRUID
(void)setruid((Uid_t)PL_uid);
(void)setruid((Uid_t)new_uid);
#else
#ifdef HAS_SETREUID
(void)setreuid((Uid_t)PL_uid, (Uid_t)-1);
(void)setreuid((Uid_t)new_uid, (Uid_t)-1);
#else
#ifdef HAS_SETRESUID
(void)setresuid((Uid_t)PL_uid, (Uid_t)-1, (Uid_t)-1);
(void)setresuid((Uid_t)new_uid, (Uid_t)-1, (Uid_t)-1);
#else
if (PL_uid == PL_euid) { /* special case $< = $> */
if (new_uid == PerlProc_geteuid()) { /* special case $< = $> */
#ifdef PERL_DARWIN
/* workaround for Darwin's setuid peculiarity, cf [perl #24122] */
if (PL_uid != 0 && PerlProc_getuid() == 0)
if (new_uid != 0 && PerlProc_getuid() == 0)
(void)PerlProc_setuid(0);
#endif
(void)PerlProc_setuid(PL_uid);
(void)PerlProc_setuid(new_uid);
} else {
PL_uid = PerlProc_getuid();
Perl_croak(aTHX_ "setruid() not implemented");
}
#endif
#endif
#endif
PL_uid = PerlProc_getuid();
break;
}
case '>':
PL_euid = SvIV(sv);
{
const UV new_euid = SvIV(sv);
PL_delaymagic_euid = new_euid;
if (PL_delaymagic) {
PL_delaymagic |= DM_EUID;
break; /* don't do magic till later */
}
#ifdef HAS_SETEUID
(void)seteuid((Uid_t)PL_euid);
(void)seteuid((Uid_t)new_euid);
#else
#ifdef HAS_SETREUID
(void)setreuid((Uid_t)-1, (Uid_t)PL_euid);
(void)setreuid((Uid_t)-1, (Uid_t)new_euid);
#else
#ifdef HAS_SETRESUID
(void)setresuid((Uid_t)-1, (Uid_t)PL_euid, (Uid_t)-1);
(void)setresuid((Uid_t)-1, (Uid_t)new_euid, (Uid_t)-1);
#else
if (PL_euid == PL_uid) /* special case $> = $< */
PerlProc_setuid(PL_euid);
if (new_euid == PerlProc_getuid()) /* special case $> = $< */
PerlProc_setuid(my_euid);
else {
PL_euid = PerlProc_geteuid();
Perl_croak(aTHX_ "seteuid() not implemented");
}
#endif
#endif
#endif
PL_euid = PerlProc_geteuid();
break;
}
case '(':
PL_gid = SvIV(sv);
{
const UV new_gid = SvIV(sv);
PL_delaymagic_gid = new_gid;
if (PL_delaymagic) {
PL_delaymagic |= DM_RGID;
break; /* don't do magic till later */
}
#ifdef HAS_SETRGID
(void)setrgid((Gid_t)PL_gid);
(void)setrgid((Gid_t)new_gid);
#else
#ifdef HAS_SETREGID
(void)setregid((Gid_t)PL_gid, (Gid_t)-1);
(void)setregid((Gid_t)new_gid, (Gid_t)-1);
#else
#ifdef HAS_SETRESGID
(void)setresgid((Gid_t)PL_gid, (Gid_t)-1, (Gid_t) -1);
(void)setresgid((Gid_t)new_gid, (Gid_t)-1, (Gid_t) -1);
#else
if (PL_gid == PL_egid) /* special case $( = $) */
(void)PerlProc_setgid(PL_gid);
if (new_gid == PerlProc_getegid()) /* special case $( = $) */
(void)PerlProc_setgid(new_gid);
else {
PL_gid = PerlProc_getgid();
Perl_croak(aTHX_ "setrgid() not implemented");
}
#endif
#endif
#endif
PL_gid = PerlProc_getgid();
break;
}
case ')':
{
UV new_egid;
#ifdef HAS_SETGROUPS
{
const char *p = SvPV_const(sv, len);
Expand All @@ -2893,7 +2898,7 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)

while (isSPACE(*p))
++p;
PL_egid = Atol(p);
new_egid = Atol(p);
for (i = 0; i < maxgrp; ++i) {
while (*p && !isSPACE(*p))
++p;
Expand All @@ -2912,32 +2917,32 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)
Safefree(gary);
}
#else /* HAS_SETGROUPS */
PL_egid = SvIV(sv);
new_egid = SvIV(sv);
#endif /* HAS_SETGROUPS */
PL_delaymagic_egid = new_egid;
if (PL_delaymagic) {
PL_delaymagic |= DM_EGID;
break; /* don't do magic till later */
}
#ifdef HAS_SETEGID
(void)setegid((Gid_t)PL_egid);
(void)setegid((Gid_t)new_egid);
#else
#ifdef HAS_SETREGID
(void)setregid((Gid_t)-1, (Gid_t)PL_egid);
(void)setregid((Gid_t)-1, (Gid_t)new_egid);
#else
#ifdef HAS_SETRESGID
(void)setresgid((Gid_t)-1, (Gid_t)PL_egid, (Gid_t)-1);
(void)setresgid((Gid_t)-1, (Gid_t)new_egid, (Gid_t)-1);
#else
if (PL_egid == PL_gid) /* special case $) = $( */
(void)PerlProc_setgid(PL_egid);
if (new_egid == PerlProc_getgid()) /* special case $) = $( */
(void)PerlProc_setgid(new_egid);
else {
PL_egid = PerlProc_getegid();
Perl_croak(aTHX_ "setegid() not implemented");
}
#endif
#endif
#endif
PL_egid = PerlProc_getegid();
break;
}
case ':':
PL_chopset = SvPV_force(sv,len);
break;
Expand Down
35 changes: 19 additions & 16 deletions perl.c
View file
Expand Up @@ -3754,13 +3754,18 @@ S_validate_suid(pTHX_ PerlIO *rsfp)
{
PERL_ARGS_ASSERT_VALIDATE_SUID;

if (PL_euid != PL_uid || PL_egid != PL_gid) { /* (suidperl doesn't exist, in fact) */
const UV my_uid = PerlProc_getuid();
const UV my_euid = PerlProc_geteuid();
const UV my_gid = PerlProc_getgid();
const UV my_egid = PerlProc_getegid();

if (my_euid != my_uid || my_egid != my_gid) { /* (suidperl doesn't exist, in fact) */
dVAR;

PerlLIO_fstat(PerlIO_fileno(rsfp),&PL_statbuf); /* may be either wrapped or real suid */
if ((PL_euid != PL_uid && PL_euid == PL_statbuf.st_uid && PL_statbuf.st_mode & S_ISUID)
if ((my_euid != my_uid && my_euid == PL_statbuf.st_uid && PL_statbuf.st_mode & S_ISUID)
||
(PL_egid != PL_gid && PL_egid == PL_statbuf.st_gid && PL_statbuf.st_mode & S_ISGID)
(my_egid != my_gid && my_egid == PL_statbuf.st_gid && PL_statbuf.st_mode & S_ISGID)
)
if (!PL_do_undump)
Perl_croak(aTHX_ "YOU HAVEN'T DISABLED SET-ID SCRIPTS IN THE KERNEL YET!\n\
Expand Down Expand Up @@ -3804,17 +3809,14 @@ STATIC void
S_init_ids(pTHX)
{
dVAR;
PL_uid = PerlProc_getuid();
PL_euid = PerlProc_geteuid();
PL_gid = PerlProc_getgid();
PL_egid = PerlProc_getegid();
#ifdef VMS
PL_uid |= PL_gid << 16;
PL_euid |= PL_egid << 16;
#endif
const UV my_uid = PerlProc_getuid();
const UV my_euid = PerlProc_geteuid();
const UV my_gid = PerlProc_getgid();
const UV my_egid = PerlProc_getegid();

/* Should not happen: */
CHECK_MALLOC_TAINT(PL_uid && (PL_euid != PL_uid || PL_egid != PL_gid));
PL_tainting |= (PL_uid && (PL_euid != PL_uid || PL_egid != PL_gid));
CHECK_MALLOC_TAINT(my_uid && (my_euid != my_uid || my_egid != my_gid));
PL_tainting |= (my_uid && (my_euid != my_uid || my_egid != my_gid));
/* BUG */
/* PSz 27 Feb 04
* Should go by suidscript, not uid!=euid: why disallow
Expand Down Expand Up @@ -3880,9 +3882,9 @@ S_forbid_setid(pTHX_ const char flag, const bool suidscript) /* g */
}

#ifdef SETUID_SCRIPTS_ARE_SECURE_NOW
if (PL_euid != PL_uid)
if (PerlProc_getuid() != PerlProc_geteuid())
Perl_croak(aTHX_ "No %s allowed while running setuid", message);
if (PL_egid != PL_gid)
if (PerlProc_getgid() != PerlProc_getegid())
Perl_croak(aTHX_ "No %s allowed while running setgid", message);
#endif /* SETUID_SCRIPTS_ARE_SECURE_NOW */
if (suidscript)
Expand Down Expand Up @@ -4566,7 +4568,8 @@ S_mayberelocate(pTHX_ const char *const dir, STRLEN len, U32 flags)
/* And this is the new libdir. */
libdir = tempsv;
if (PL_tainting &&
(PL_uid != PL_euid || PL_gid != PL_egid)) {
(PerlProc_getuid() != PerlProc_geteuid() ||
PerlProc_getgid() != PerlProc_getegid())) {
/* Need to taint relocated paths if running set ID */
SvTAINTED_on(libdir);
}
Expand Down
4 changes: 3 additions & 1 deletion perlio.c
View file
Expand Up @@ -458,7 +458,9 @@ PerlIO_debug(const char *fmt, ...)
dSYS;
va_start(ap, fmt);
if (!PL_perlio_debug_fd) {
if (!PL_tainting && PL_uid == PL_euid && PL_gid == PL_egid) {
if (!PL_tainting &&
PerlProc_getuid() == PerlProc_geteuid() &&
PerlProc_getgid() == PerlProc_getegid()) {
const char * const s = PerlEnv_getenv("PERLIO_DEBUG");
if (s && *s)
PL_perlio_debug_fd
Expand Down

0 comments on commit 985213f

Please sign in to comment.