diff options
| -rw-r--r-- | embedvar.h | 4 | ||||
| -rw-r--r-- | intrpvar.h | 5 | ||||
| -rw-r--r-- | perl.c | 529 | ||||
| -rw-r--r-- | perl.h | 13 | ||||
| -rw-r--r-- | perlapi.h | 4 |
5 files changed, 427 insertions, 128 deletions
diff --git a/embedvar.h b/embedvar.h index f7cc10aab7..2f404b51ad 100644 --- a/embedvar.h +++ b/embedvar.h @@ -246,6 +246,7 @@ #define PL_exitlistlen (vTHX->Iexitlistlen) #define PL_expect (vTHX->Iexpect) #define PL_fdpid (vTHX->Ifdpid) +#define PL_fdscript (vTHX->Ifdscript) #define PL_filemode (vTHX->Ifilemode) #define PL_forkprocess (vTHX->Iforkprocess) #define PL_formfeed (vTHX->Iformfeed) @@ -393,6 +394,7 @@ #define PL_sublex_info (vTHX->Isublex_info) #define PL_subline (vTHX->Isubline) #define PL_subname (vTHX->Isubname) +#define PL_suidscript (vTHX->Isuidscript) #define PL_sv_arenaroot (vTHX->Isv_arenaroot) #define PL_sv_count (vTHX->Isv_count) #define PL_sv_no (vTHX->Isv_no) @@ -547,6 +549,7 @@ #define PL_Iexitlistlen PL_exitlistlen #define PL_Iexpect PL_expect #define PL_Ifdpid PL_fdpid +#define PL_Ifdscript PL_fdscript #define PL_Ifilemode PL_filemode #define PL_Iforkprocess PL_forkprocess #define PL_Iformfeed PL_formfeed @@ -694,6 +697,7 @@ #define PL_Isublex_info PL_sublex_info #define PL_Isubline PL_subline #define PL_Isubname PL_subname +#define PL_Isuidscript PL_suidscript #define PL_Isv_arenaroot PL_sv_arenaroot #define PL_Isv_count PL_sv_count #define PL_Isv_no PL_sv_no diff --git a/intrpvar.h b/intrpvar.h index ee805e6a07..a477777034 100644 --- a/intrpvar.h +++ b/intrpvar.h @@ -530,6 +530,11 @@ PERLVARI(Irehash_seed, UV, 0) /* 582 hash initializer */ PERLVARI(Irehash_seed_set, bool, FALSE) /* 582 hash initialized? */ +/* These two variables are needed to preserve 5.8.x bincompat because we can't + change function prototypes of two exported functions. Probably should be + taken out of blead soon, and relevant prototypes changed. */ +PERLVARI(Ifdscript, int, -1) /* fd for script */ +PERLVARI(Isuidscript, int, -1) /* fd for suid script */ /* New variables must be added to the very end, before this comment, * for binary compatibility (the offsets of the old members must not change). * (Don't forget to add your variable also to perl_clone()!) @@ -12,6 +12,65 @@ * "A ship then new they built for him/of mithril and of elven glass" --Bilbo */ +/* PSz 12 Nov 03 + * + * Be proud that perl(1) may proclaim: + * Setuid Perl scripts are safer than C programs ... + * Do not abandon (deprecate) suidperl. Do not advocate C wrappers. + * + * The flow was: perl starts, notices script is suid, execs suidperl with same + * arguments; suidperl opens script, checks many things, sets itself with + * right UID, execs perl with similar arguments but with script pre-opened on + * /dev/fd/xxx; perl checks script is as should be and does work. This was + * insecure: see perlsec(1) for many problems with this approach. + * + * The "correct" flow should be: perl starts, opens script and notices it is + * suid, checks many things, execs suidperl with similar arguments but with + * script on /dev/fd/xxx; suidperl checks script and /dev/fd/xxx object are + * same, checks arguments match #! line, sets itself with right UID, execs + * perl with same arguments; perl checks many things and does work. + * + * (Opening the script in perl instead of suidperl, we "lose" scripts that + * are readable to the target UID but not to the invoker. Where did + * unreadable scripts work anyway?) + * + * For now, suidperl and perl are pretty much the same large and cumbersome + * program, so suidperl can check its argument list (see comments elsewhere). + * + * References: + * Original bug report: + * http://bugs.perl.org/index.html?req=bug_id&bug_id=20010322.218 + * http://rt.perl.org/rt2/Ticket/Display.html?id=6511 + * Comments and discussion with Debian: + * http://bugs.debian.org/203426 + * http://bugs.debian.org/220486 + * Debian Security Advisory DSA 431-1 (does not fully fix problem): + * http://www.debian.org/security/2004/dsa-431 + * CVE candidate: + * http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2003-0618 + * Previous versions of this patch sent to perl5-porters: + * http://www.mail-archive.com/perl5-porters@perl.org/msg71953.html + * http://www.mail-archive.com/perl5-porters@perl.org/msg75245.html + * http://www.mail-archive.com/perl5-porters@perl.org/msg75563.html + * http://www.mail-archive.com/perl5-porters@perl.org/msg75635.html + * +Paul Szabo - psz@maths.usyd.edu.au http://www.maths.usyd.edu.au:8000/u/psz/ +School of Mathematics and Statistics University of Sydney 2006 Australia + * + */ +/* PSz 13 Nov 03 + * Use truthful, neat, specific error messages. + * Cannot always hide the truth; security must not depend on doing so. + */ + +/* PSz 18 Feb 04 + * Use global(?), thread-local fdscript for easier checks. + * (I do not understand how we could possibly get a thread race: + * do not all threads go through the same initialization? Or in + * fact, are not threads started only after we get the script and + * so know what to do? Oh well, make things super-safe...) + */ + #include "EXTERN.h" #define PERL_IN_PERL_C #include "perl.h" @@ -49,7 +108,7 @@ static I32 read_e_script(pTHX_ int idx, SV *buf_sv, int maxlen); #ifndef DOSUID #define DOSUID #endif -#endif +#endif /* IAMSUID */ #ifdef SETUID_SCRIPTS_ARE_SECURE_NOW #ifdef DOSUID @@ -933,7 +992,7 @@ perl_parse(pTHXx_ XSINIT_t xsinit, int argc, char **argv, char **env) #undef IAMSUID Perl_croak(aTHX_ "suidperl is no longer needed since the kernel can now execute\n\ setuid perl scripts securely.\n"); -#endif +#endif /* IAMSUID */ #endif #if defined(USE_HASH_SEED) || defined(USE_HASH_SEED_EXPLICIT) @@ -1124,13 +1183,16 @@ S_parse_body(pTHX_ char **env, XSINIT_t xsinit) int argc = PL_origargc; char **argv = PL_origargv; char *scriptname = NULL; - int fdscript = -1; VOL bool dosearch = FALSE; char *validarg = ""; register SV *sv; register char *s; char *cddir = Nullch; +/* PSz 18 Feb 04 fdscript now global, keep from confusion */ + int dummy_fdscript = -1; + PL_fdscript = -1; + PL_suidscript = -1; sv_setpvn(PL_linestr,"",0); sv = newSVpvn("",0); /* first used for -I flags */ SAVEFREESV(sv); @@ -1144,6 +1206,12 @@ S_parse_body(pTHX_ char **env, XSINIT_t xsinit) validarg = " PHOOEY "; else validarg = argv[0]; + /* + * Can we rely on the kernel to start scripts with argv[1] set to + * contain all #! line switches (the whole line)? (argv[0] is set to + * the interpreter name, argv[2] to the script name; argv[3] and + * above may contain other arguments.) + */ #endif s = argv[0]+1; reswitch: @@ -1199,8 +1267,7 @@ S_parse_body(pTHX_ char **env, XSINIT_t xsinit) if (argv[1] && !strcmp(argv[1], "Dev:Pseudo")) break; #endif - if (PL_euid != PL_uid || PL_egid != PL_gid) - Perl_croak(aTHX_ "No -e allowed in setuid scripts"); + forbid_setid("-e"); if (!PL_e_script) { PL_e_script = newSVpvn("",0); filter_add(read_e_script, NULL); @@ -1432,9 +1499,9 @@ print \" \\@INC:\\n @INC\\n\";"); init_perllib(); - open_script(scriptname,dosearch,sv,&fdscript); + open_script(scriptname,dosearch,sv,&dummy_fdscript); - validate_suid(validarg, scriptname,fdscript); + validate_suid(validarg, scriptname,dummy_fdscript); #ifndef PERL_MICRO #if defined(SIGCHLD) || defined(SIGCLD) @@ -2865,15 +2932,19 @@ S_init_main_stash(pTHX) sv_setpvn(get_sv("/", TRUE), "\n", 1); } +/* PSz 18 Nov 03 fdscript now global but do not change prototype */ STATIC void -S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) +S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *dummy_fdscript) { +#ifndef IAMSUID char *quote; char *code; char *cpp_discard_flag; char *perl; +#endif - *fdscript = -1; + PL_fdscript = -1; + PL_suidscript = -1; if (PL_e_script) { PL_origfilename = savepv("-e"); @@ -2884,10 +2955,30 @@ S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) if (strnEQ(scriptname, "/dev/fd/", 8) && isDIGIT(scriptname[8]) ) { char *s = scriptname + 8; - *fdscript = atoi(s); + PL_fdscript = atoi(s); while (isDIGIT(*s)) s++; if (*s) { + /* PSz 18 Feb 04 + * Tell apart "normal" usage of fdscript, e.g. + * with bash on FreeBSD: + * perl <( echo '#!perl -DA'; echo 'print "$0\n"') + * from usage in suidperl. + * Does any "normal" usage leave garbage after the number??? + * Is it a mistake to use a similar /dev/fd/ construct for + * suidperl? + */ + PL_suidscript = 1; + /* PSz 20 Feb 04 + * Be supersafe and do some sanity-checks. + * Still, can we be sure we got the right thing? + */ + if (*s != '/') { + Perl_croak(aTHX_ "Wrong syntax (suid) fd script name \"%s\"\n", s); + } + if (! *(s+1)) { + Perl_croak(aTHX_ "Missing (suid) fd script name\n"); + } scriptname = savepv(s + 1); Safefree(PL_origfilename); PL_origfilename = scriptname; @@ -2899,14 +2990,28 @@ S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) CopFILE_set(PL_curcop, PL_origfilename); if (strEQ(PL_origfilename,"-")) scriptname = ""; - if (*fdscript >= 0) { - PL_rsfp = PerlIO_fdopen(*fdscript,PERL_SCRIPT_MODE); + if (PL_fdscript >= 0) { + PL_rsfp = PerlIO_fdopen(PL_fdscript,PERL_SCRIPT_MODE); # if defined(HAS_FCNTL) && defined(F_SETFD) if (PL_rsfp) /* ensure close-on-exec */ fcntl(PerlIO_fileno(PL_rsfp),F_SETFD,1); # endif } +#ifdef IAMSUID + else { + Perl_croak(aTHX_ "suidperl needs fd script\n"); +/* PSz 11 Nov 03 + * Do not open (or do other fancy stuff) while setuid. + * Perl does the open, and hands script to suidperl on a fd; + * suidperl only does some checks, sets up UIDs and re-execs + * perl with that fd as it has always done. + */ + } + if (PL_suidscript != 1) { + Perl_croak(aTHX_ "suidperl needs (suid) fd script\n"); + } +#else /* IAMSUID */ else if (PL_preprocess) { char *cpp_cfg = CPPSTDIN; SV *cpp = newSVpvn("",0); @@ -2963,25 +3068,6 @@ S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) cpp_discard_flag, sv, CPPMINUS); PL_doextract = FALSE; -# ifdef IAMSUID /* actually, this is caught earlier */ - if (PL_euid != PL_uid && !PL_euid) { /* if running suidperl */ -# ifdef HAS_SETEUID - (void)seteuid(PL_uid); /* musn't stay setuid root */ -# else -# ifdef HAS_SETREUID - (void)setreuid((Uid_t)-1, PL_uid); -# else -# ifdef HAS_SETRESUID - (void)setresuid((Uid_t)-1, PL_uid, (Uid_t)-1); -# else - PerlProc_setuid(PL_uid); -# endif -# endif -# endif - if (PerlProc_geteuid() != PL_uid) - Perl_croak(aTHX_ "Can't do seteuid!\n"); - } -# endif /* IAMSUID */ DEBUG_P(PerlIO_printf(Perl_debug_log, "PL_preprocess: cmd=\"%s\"\n", @@ -3003,31 +3089,11 @@ S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) fcntl(PerlIO_fileno(PL_rsfp),F_SETFD,1); # endif } +#endif /* IAMSUID */ if (!PL_rsfp) { -# ifdef DOSUID -# ifndef IAMSUID /* in case script is not readable before setuid */ - if (PL_euid && - PerlLIO_stat(CopFILE(PL_curcop),&PL_statbuf) >= 0 && - PL_statbuf.st_mode & (S_ISUID|S_ISGID)) - { - /* try again */ - PERL_FPU_PRE_EXEC - PerlProc_execv(Perl_form(aTHX_ "%s/sperl"PERL_FS_VER_FMT, - BIN_EXP, (int)PERL_REVISION, - (int)PERL_VERSION, - (int)PERL_SUBVERSION), PL_origargv); - PERL_FPU_POST_EXEC - Perl_croak(aTHX_ "Can't do setuid\n"); - } -# endif -# endif -# ifdef IAMSUID - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); -# else +/* PSz 16 Sep 03 Keep neat error message */ Perl_croak(aTHX_ "Can't open perl script \"%s\": %s\n", CopFILE(PL_curcop), Strerror(errno)); -# endif } } @@ -3042,8 +3108,19 @@ S_open_script(pTHX_ char *scriptname, bool dosearch, SV *sv, int *fdscript) STATIC int S_fd_on_nosuid_fs(pTHX_ int fd) { +/* PSz 27 Feb 04 + * We used to do this as "plain" user (after swapping UIDs with setreuid); + * but is needed also on machines without setreuid. + * Seems safe enough to run as root. + */ int check_okay = 0; /* able to do all the required sys/libcalls */ int on_nosuid = 0; /* the fd is on a nosuid fs */ + /* PSz 12 Nov 03 + * Need to check noexec also: nosuid might not be set, the average + * sysadmin would say that nosuid is irrelevant once he sets noexec. + */ + int on_noexec = 0; /* the fd is on a noexec fs */ + /* * Preferred order: fstatvfs(), fstatfs(), ustat()+getmnt(), getmntent(). * fstatvfs() is UNIX98. @@ -3062,10 +3139,16 @@ S_fd_on_nosuid_fs(pTHX_ int fd) check_okay = fstatvfs(fd, &stfs) == 0; on_nosuid = check_okay && (stfs.f_flag & ST_NOSUID); +#ifdef ST_NOEXEC + /* ST_NOEXEC certainly absent on AIX 5.1, and doesn't seem to be documented + on platforms where it is present. */ + on_noexec = check_okay && (stfs.f_flag & ST_NOEXEC); +#endif # endif /* fstatvfs */ # if !defined(FD_ON_NOSUID_CHECK_OKAY) && \ defined(PERL_MOUNT_NOSUID) && \ + defined(PERL_MOUNT_NOEXEC) && \ defined(HAS_FSTATFS) && \ defined(HAS_STRUCT_STATFS) && \ defined(HAS_STRUCT_STATFS_F_FLAGS) @@ -3074,10 +3157,12 @@ S_fd_on_nosuid_fs(pTHX_ int fd) check_okay = fstatfs(fd, &stfs) == 0; on_nosuid = check_okay && (stfs.f_flags & PERL_MOUNT_NOSUID); + on_noexec = check_okay && (stfs.f_flags & PERL_MOUNT_NOEXEC); # endif /* fstatfs */ # if !defined(FD_ON_NOSUID_CHECK_OKAY) && \ defined(PERL_MOUNT_NOSUID) && \ + defined(PERL_MOUNT_NOEXEC) && \ defined(HAS_FSTAT) && \ defined(HAS_USTAT) && \ defined(HAS_GETMNT) && \ @@ -3100,6 +3185,7 @@ S_fd_on_nosuid_fs(pTHX_ int fd) fdst.st_dev == fsd.fd_req.dev) { check_okay = 1; on_nosuid = fsd.fd_req.flags & PERL_MOUNT_NOSUID; + on_noexec = fsd.fd_req.flags & PERL_MOUNT_NOEXEC; } } } @@ -3110,7 +3196,8 @@ S_fd_on_nosuid_fs(pTHX_ int fd) # if !defined(FD_ON_NOSUID_CHECK_OKAY) && \ defined(HAS_GETMNTENT) && \ defined(HAS_HASMNTOPT) && \ - defined(MNTOPT_NOSUID) + defined(MNTOPT_NOSUID) && \ + defined(MNTOPT_NOEXEC) # define FD_ON_NOSUID_CHECK_OKAY FILE *mtab = fopen("/etc/mtab", "r"); struct mntent *entry; @@ -3125,6 +3212,8 @@ S_fd_on_nosuid_fs(pTHX_ int fd) check_okay = 1; if (hasmntopt(entry, MNTOPT_NOSUID)) on_nosuid = 1; + if (hasmntopt(entry, MNTOPT_NOEXEC)) + on_noexec = 1; break; } /* A single fs may well fail its stat(). */ } @@ -3134,17 +3223,22 @@ S_fd_on_nosuid_fs(pTHX_ int fd) # endif /* getmntent+hasmntopt */ if (!check_okay) - Perl_croak(aTHX_ "Can't check filesystem of script \"%s\" for nosuid", PL_origfilename); - return on_nosuid; + Perl_croak(aTHX_ "Can't check filesystem of script \"%s\" for nosuid/noexec", PL_origfilename); + if (on_nosuid) + Perl_croak(aTHX_ "Setuid script \"%s\" on nosuid filesystem", PL_origfilename); + if (on_noexec) + Perl_croak(aTHX_ "Setuid script \"%s\" on noexec filesystem", PL_origfilename); + return ((!check_okay) || on_nosuid || on_noexec); } #endif /* IAMSUID */ +/* PSz 18 Nov 03 fdscript now global but do not change prototype */ STATIC void -S_validate_suid(pTHX_ char *validarg, char *scriptname, int fdscript) +S_validate_suid(pTHX_ char *validarg, char *scriptname, int dummy_fdscript) { #ifdef IAMSUID - int which; -#endif + /* int which; */ +#endif /* IAMSUID */ /* do we need to emulate setuid on scripts? */ @@ -3160,6 +3254,13 @@ S_validate_suid(pTHX_ char *validarg, char *scriptname, int fdscript) * uid. We don't just make perl setuid root because that loses the * effective uid we had before invoking perl, if it was different from the * uid. + * PSz 27 Feb 04 + * Description/comments above do not match current workings: + * suidperl must be hardlinked to sperlN.NNN (that is what we exec); + * suidperl called with script open and name changed to /dev/fd/N/X; + * suidperl croaks if script is not setuid; + * making perl setuid would be a huge security risk (and yes, that + * would lose any euid we might have had). * * DOSUID must be defined in both perl and suidperl, and IAMSUID must * be defined in suidperl only. suidperl must be setuid root. The @@ -3171,12 +3272,33 @@ S_validate_suid(pTHX_ char *validarg, char *scriptname, int fdscript) if (PerlLIO_fstat(PerlIO_fileno(PL_rsfp),&PL_statbuf) < 0) /* normal stat is insecure */ Perl_croak(aTHX_ "Can't stat script \"%s\"",PL_origfilename); - if (fdscript < 0 && PL_statbuf.st_mode & (S_ISUID|S_ISGID)) { + if (PL_statbuf.st_mode & (S_ISUID|S_ISGID)) { I32 len; STRLEN n_a; #ifdef IAMSUID -#ifndef HAS_SETREUID + if (PL_fdscript < 0 || PL_suidscript != 1) + Perl_croak(aTHX_ "Need (suid) fdscript in suidperl\n"); /* We already checked this */ + /* PSz 11 Nov 03 + * Since the script is opened by perl, not suidperl, some of these + * checks are superfluous. Leaving them in probably does not lower + * security(?!). + */ + /* PSz 27 Feb 04 + * Do checks even for systems with no HAS_SETREUID. + * We used to swap, then re-swap UIDs with +#ifdef HAS_SETREUID + if (setreuid(PL_euid,PL_uid) < 0 + || PerlProc_getuid() != PL_euid || PerlProc_geteuid() != PL_uid) + Perl_croak(aTHX_ "Can't swap uid and euid"); +#endif +#ifdef HAS_SETREUID + if (setreuid(PL_uid,PL_euid) < 0 + || PerlProc_getuid() != PL_uid || PerlProc_geteuid() != PL_euid) + Perl_croak(aTHX_ "Can't reswap uid and euid"); +#endif + */ + /* On this access check to make sure the directories are readable, * there is actually a small window that the user could use to make * filename point to an accessible directory. So there is a faint @@ -3184,79 +3306,89 @@ S_validate_suid(pTHX_ char *validarg, char *scriptname, int fdscript) * non-accessible directory. I don't know what to do about that. * But I don't think it's too important. The manual lies when * it says access() is useful in setuid programs. + * + * So, access() is pretty useless... but not harmful... do anyway. */ if (PerlLIO_access(CopFILE(PL_curcop),1)) { /*double check*/ - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); + Perl_croak(aTHX_ "Can't access() script\n"); } -#else + /* If we can swap euid and uid, then we can determine access rights * with a simple stat of the file, and then compare device and * inode to make sure we did stat() on the same file we opened. * Then we just have to make sure he or she can execute it. + * + * PSz 24 Feb 04 + * As the script is opened by perl, not suidperl, we do not need to + * care much about access rights. + * + * The 'script changed' check is needed, or we can get lied to + * about $0 with e.g. + * suidperl /dev/fd/4//bin/x 4<setuidscript + * Without HAS_SETREUID, is it safe to stat() as root? + * + * Are there any operating systems that pass /dev/fd/xxx for setuid + * scripts, as suggested/described in perlsec(1)? Surely they do not + * pass the script name as we do, so the "script changed" test would + * fail for them... but we never get here with + * SETUID_SCRIPTS_ARE_SECURE_NOW defined. + * + * This is one place where we must "lie" about return status: not + * say if the stat() failed. We are doing this as root, and could + * be tricked into reporting existence or not of files that the + * "plain" user cannot even see. */ { Stat_t tmpstatbuf; - - if ( -#ifdef HAS_SETREUID - setreuid(PL_euid,PL_uid) < 0 -#else -# if HAS_SETRESUID - setresuid(PL_euid,PL_uid,(Uid_t)-1) < 0 -# endif -#endif - || PerlProc_getuid() != PL_euid || PerlProc_geteuid() != PL_uid) - Perl_croak(aTHX_ "Can't swap uid and euid"); /* really paranoid */ - if (PerlLIO_stat(CopFILE(PL_curcop),&tmpstatbuf) < 0) { - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); /* testing full pathname here */ - } -#if defined(IAMSUID) && !defined(NO_NOSUID_CHECK) - if (fd_on_nosuid_fs(PerlIO_fileno(PL_rsfp))) { - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); - } -#endif - if (tmpstatbuf.st_dev != PL_statbuf.st_dev || + if (PerlLIO_stat(CopFILE(PL_curcop),&tmpstatbuf) < 0 || + tmpstatbuf.st_dev != PL_statbuf.st_dev || tmpstatbuf.st_ino != PL_statbuf.st_ino) { - (void)PerlIO_close(PL_rsfp); - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); + Perl_croak(aTHX_ "Setuid script changed\n"); } - if ( -#ifdef HAS_SETREUID - setreuid(PL_uid,PL_euid) < 0 -#else -# if defined(HAS_SETRESUID) - setresuid(PL_uid,PL_euid,(Uid_t)-1) < 0 -# endif -#endif - || PerlProc_getuid() != PL_uid || PerlProc_geteuid() != PL_euid) - Perl_croak(aTHX_ "Can't reswap uid and euid"); - if (!cando(S_IXUSR,FALSE,&PL_statbuf)) /* can real uid exec? */ - Perl_croak(aTHX_ "Permission denied\n"); + } -#endif /* HAS_SETREUID */ + if (!cando(S_IXUSR,FALSE,&PL_statbuf)) /* can real uid exec? */ + Perl_croak(aTHX_ "Real UID cannot exec script\n"); + + /* PSz 27 Feb 04 + * We used to do this check as the "plain" user (after swapping + * UIDs). But the check for nosuid and noexec filesystem is needed, + * and should be done even without HAS_SETREUID. (Maybe those + * operating systems do not have such mount options anyway...) + * Seems safe enough to do as root. + */ +#if !defined(NO_NOSUID_CHECK) + if (fd_on_nosuid_fs(PerlIO_fileno(PL_rsfp))) { + Perl_croak(aTHX_ "Setuid script on nosuid or noexec filesystem\n"); + } +#endif #endif /* IAMSUID */ if (!S_ISREG(PL_statbuf.st_mode)) { - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); + Perl_croak(aTHX_ "Setuid script not plain file\n"); } if (PL_statbuf.st_mode & S_IWOTH) Perl_croak(aTHX_ "Setuid/gid script is writable by world"); PL_doswitches = FALSE; /* -s is insecure in suid */ + /* PSz 13 Nov 03 But -s was caught elsewhere ... so unsetting it here is useless(?!) */ CopLINE_inc(PL_curcop); if (sv_gets(PL_linestr, PL_rsfp, 0) == Nullch || strnNE(SvPV(PL_linestr,n_a),"#!",2) ) /* required even on Sys V */ Perl_croak(aTHX_ "No #! line"); s = SvPV(PL_linestr,n_a)+2; - if (*s == ' ') s++; - while (!isSPACE(*s)) s++; + /* PSz 27 Feb 04 */ + /* Sanity check on line length */ + if (strlen(s) < 1 || strlen(s) > 4000) + Perl_croak(aTHX_ "Very long #! line"); + /* Allow more than a single space after #! */ + while (isSPACE(*s)) s++; + /* Sanity check on buffer end */ + while ((*s) && !isSPACE(*s)) s++; for (s2 = s; (s2 > SvPV(PL_linestr,n_a)+2 && (isDIGIT(s2[-1]) || strchr("._-", s2[-1]))); s2--) ; - if (strnNE(s2-4,"perl",4) && strnNE(s-9,"perl",4)) /* sanity check */ + /* Sanity check on buffer start */ + if ( (s2-4 < SvPV(PL_linestr,n_a)+2 || strnNE(s2-4,"perl",4)) && + (s-9 < SvPV(PL_linestr,n_a)+2 || strnNE(s-9,"perl",4)) ) Perl_croak(aTHX_ "Not a perl script"); while (*s == ' ' || *s == '\t') s++; /* @@ -3264,33 +3396,101 @@ S_validate_suid(pTHX_ char *validarg, char *scriptname, int fdscript) * mentioning suidperl explicitly, but they may not add any strange * arguments beyond what #! says if they do invoke suidperl that way. */ + /* + * The way validarg was set up, we rely on the kernel to start + * scripts with argv[1] set to contain all #! line switches (the + * whole line). + */ + /* + * Check that we got all the arguments listed in the #! line (not + * just that there are no extraneous arguments). Might not matter + * much, as switches from #! line seem to be acted upon (also), and + * so may be checked and trapped in perl. But, security checks must + * be done in suidperl and not deferred to perl. Note that suidperl + * does not get around to parsing (and checking) the switches on + * the #! line (but execs perl sooner). + * Allow (require) a trailing newline (which may be of two + * characters on some architectures?) (but no other trailing + * whitespace). + */ len = strlen(validarg); if (strEQ(validarg," PHOOEY ") || - strnNE(s,validarg,len) || !isSPACE(s[len])) + strnNE(s,validarg,len) || !isSPACE(s[len]) || + !(strlen(s) == len+1 || (strlen(s) == len+2 && isSPACE(s[len+1])))) Perl_croak(aTHX_ "Args must match #! line"); #ifndef IAMSUID - if (PL_euid != PL_uid && (PL_statbuf.st_mode & S_ISUID) && + if (PL_fdscript < 0 && + PL_euid != PL_uid && (PL_statbuf.st_mode & S_ISUID) && PL_euid == PL_statbuf.st_uid) if (!PL_do_undump) Perl_croak(aTHX_ "YOU HAVEN'T DISABLED SET-ID SCRIPTS IN THE KERNEL YET!\n\ FIX YOUR KERNEL, OR PUT A C WRAPPER AROUND THIS SCRIPT!\n"); #endif /* IAMSUID */ - if (PL_euid) { /* oops, we're not the setuid root perl */ - (void)PerlIO_close(PL_rsfp); + if (PL_fdscript < 0 && + PL_euid) { /* oops, we're not the setuid root perl */ + /* PSz 18 Feb 04 + * When root runs a setuid script, we do not go through the same + * steps of execing sperl and then perl with fd scripts, but + * simply set up UIDs within the same perl invocation; so do + * not have the same checks (on options, whatever) that we have + * for plain users. No problem really: would have to be a script + * that does not actually work for plain users; and if root is + * foolish and can be persuaded to run such an unsafe script, he + * might run also non-setuid ones, and deserves what he gets. + * + * Or, we might drop the PL_euid check above (and rely just on + * PL_fdscript to avoid loops), and do the execs + * even for root. + */ #ifndef IAMSUID - /* try again */ + int which; + /* PSz 11 Nov 03 + * Pass fd script to suidperl. + * Exec suidperl, substituting fd script for scriptname. + * Pass script name as "subdir" of fd, which perl will grok; + * in fact will use that to distinguish this from "normal" + * usage, see comments above. + */ + PerlIO_rewind(PL_rsfp); + PerlLIO_lseek(PerlIO_fileno(PL_rsfp),(Off_t)0,0); /* just in case rewind didn't */ + /* PSz 27 Feb 04 Sanity checks on scriptname */ + if ((!scriptname) || (!*scriptname) ) { + Perl_croak(aTHX_ "No setuid script name\n"); + } + if (*scriptname == '-') { + Perl_croak(aTHX_ "Setuid script name may not begin with dash\n"); + /* Or we might confuse it with an option when replacing + * name in argument list, below (though we do pointer, not + * string, comparisons). + */ + } + for (which = 1; PL_origargv[which] && PL_origargv[which] != scriptname; which++) ; + if (!PL_origargv[which]) { + Perl_croak(aTHX_ "Can't change argv to have fd script\n"); + } + PL_origargv[which] = savepv(Perl_form(aTHX_ "/dev/fd/%d/%s", + PerlIO_fileno(PL_rsfp), PL_origargv[which])); +#if defined(HAS_FCNTL) && defined(F_SETFD) + fcntl(PerlIO_fileno(PL_rsfp),F_SETFD,0); /* ensure no close-on-exec */ +#endif PERL_FPU_PRE_EXEC PerlProc_execv(Perl_form(aTHX_ "%s/sperl"PERL_FS_VER_FMT, BIN_EXP, (int)PERL_REVISION, (int)PERL_VERSION, (int)PERL_SUBVERSION), PL_origargv); PERL_FPU_POST_EXEC -#endif - Perl_croak(aTHX_ "Can't do setuid\n"); +#endif /* IAMSUID */ + Perl_croak(aTHX_ "Can't do setuid (cannot exec sperl)\n"); } if (PL_statbuf.st_mode & S_ISGID && PL_statbuf.st_gid != PL_egid) { +/* PSz 26 Feb 04 + * This seems back to front: we try HAS_SETEGID first; if not available + * then try HAS_SETREGID; as a last chance we try HAS_SETRESGID. May be OK + * in the sense that we only want to set EGID; but are there any machines + * with either of the latter, but not the former? Same with UID, later. + */ #ifdef HAS_SETEGID (void)setegid(PL_statbuf.st_gid); #else @@ -3344,30 +3544,64 @@ FIX YOUR KERNEL, OR PUT A C WRAPPER AROUND THIS SCRIPT!\n"); } init_ids(); if (!cando(S_IXUSR,TRUE,&PL_statbuf)) - Perl_croak(aTHX_ "Permission denied\n"); /* they can't do this */ + Perl_croak(aTHX_ "Effective UID cannot exec script\n"); /* they can't do this */ } #ifdef IAMSUID - else if (PL_preprocess) + else if (PL_preprocess) /* PSz 13 Nov 03 Caught elsewhere, useless(?!) here */ Perl_croak(aTHX_ "-P not allowed for setuid/setgid script\n"); - else if (fdscript >= 0) - Perl_croak(aTHX_ "fd script not allowed in suidperl\n"); + else if (PL_fdscript < 0 || PL_suidscript != 1) + /* PSz 13 Nov 03 Caught elsewhere, useless(?!) here */ + Perl_croak(aTHX_ "(suid) fdscript needed in suidperl\n"); else { - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); +/* PSz 16 Sep 03 Keep neat error message */ + Perl_croak(aTHX_ "Script is not setuid/setgid in suidperl\n"); } /* We absolutely must clear out any saved ids here, so we */ /* exec the real perl, substituting fd script for scriptname. */ /* (We pass script name as "subdir" of fd, which perl will grok.) */ + /* + * It might be thought that using setresgid and/or setresuid (changed to + * set the saved IDs) above might obviate the need to exec, and we could + * go on to "do the perl thing". + * + * Is there such a thing as "saved GID", and is that set for setuid (but + * not setgid) execution like suidperl? Without exec, it would not be + * cleared for setuid (but not setgid) scripts (or might need a dummy + * setresgid). + * + * We need suidperl to do the exact same argument checking that perl + * does. Thus it cannot be very small; while it could be significantly + * smaller, it is safer (simpler?) to make it essentially the same + * binary as perl (but they are not identical). - Maybe could defer that + * check to the invoked perl, and suidperl be a tiny wrapper instead; + * but prefer to do thorough checks in suidperl itself. Such deferral + * would make suidperl security rely on perl, a design no-no. + * + * Setuid things should be short and simple, thus easy to understand and + * verify. They should do their "own thing", without influence by + * attackers. It may help if their internal execution flow is fixed, + * regardless of platform: it may be best to exec anyway. + * + * Suidperl should at least be conceptually simple: a wrapper only, + * never to do any real perl. Maybe we should put + * #ifdef IAMSUID + * Perl_croak(aTHX_ "Suidperl should never do real perl\n"); + * #endif + * into the perly bits. + */ PerlIO_rewind(PL_rsfp); PerlLIO_lseek(PerlIO_fileno(PL_rsfp),(Off_t)0,0); /* just in case rewind didn't */ - for (which = 1; PL_origargv[which] && PL_origargv[which] != scriptname; which++) ; - if (!PL_origargv[which]) { - errno = EPERM; - Perl_croak(aTHX_ "Permission denied\n"); - } - PL_origargv[which] = savepv(Perl_form(aTHX_ "/dev/fd/%d/%s", - PerlIO_fileno(PL_rsfp), PL_origargv[which])); + /* PSz 11 Nov 03 + * Keep original arguments: suidperl already has fd script. + */ +/* for (which = 1; PL_origargv[which] && PL_origargv[which] != scriptname; which++) ; */ +/* if (!PL_origargv[which]) { */ +/* errno = EPERM; */ +/* Perl_croak(aTHX_ "Permission denied\n"); */ +/* } */ +/* PL_origargv[which] = savepv(Perl_form(aTHX_ "/dev/fd/%d/%s", */ +/* PerlIO_fileno(PL_rsfp), PL_origargv[which])); */ #if defined(HAS_FCNTL) && defined(F_SETFD) fcntl(PerlIO_fileno(PL_rsfp),F_SETFD,0); /* ensure no close-on-exec */ #endif @@ -3376,7 +3610,7 @@ FIX YOUR KERNEL, OR PUT A C WRAPPER AROUND THIS SCRIPT!\n"); (int)PERL_REVISION, (int)PERL_VERSION, (int)PERL_SUBVERSION), PL_origargv);/* try again */ PERL_FPU_POST_EXEC - Perl_croak(aTHX_ "Can't do setuid\n"); + Perl_croak(aTHX_ "Can't do setuid (suidperl cannot exec perl)\n"); #endif /* IAMSUID */ #else /* !DOSUID */ if (PL_euid != PL_uid || PL_egid != PL_gid) { /* (suidperl doesn't exist, in fact) */ @@ -3477,6 +3711,15 @@ S_init_ids(pTHX) /* 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)); + /* BUG */ + /* PSz 27 Feb 04 + * Should go by suidscript, not uid!=euid: why disallow + * system("ls") in scripts run from setuid things? + * Or, is this run before we check arguments and set suidscript? + * What about SETUID_SCRIPTS_ARE_SECURE_NOW: could we use fdscript then? + * (We never have suidscript, can we be sure to have fdscript?) + * Or must then go by UID checks? See comments in forbid_setid also. + */ } /* This is used very early in the lifetime of the program, @@ -3518,10 +3761,40 @@ Perl_doing_taint(int argc, char *argv[], char *envp[]) STATIC void S_forbid_setid(pTHX_ char *s) { +#ifdef SETUID_SCRIPTS_ARE_SECURE_NOW if (PL_euid != PL_uid) Perl_croak(aTHX_ "No %s allowed while running setuid", s); if (PL_egid != PL_gid) Perl_croak(aTHX_ "No %s allowed while running setgid", s); +#endif /* SETUID_SCRIPTS_ARE_SECURE_NOW */ + /* PSz 29 Feb 04 + * Checks for UID/GID above "wrong": why disallow + * perl -e 'print "Hello\n"' + * from within setuid things?? Simply drop them: replaced by + * fdscript/suidscript and #ifdef IAMSUID checks below. + * + * This may be too late for command-line switches. Will catch those on + * the #! line, after finding the script name and setting up + * fdscript/suidscript. Note that suidperl does not get around to + * parsing (and checking) the switches on the #! line, but checks that + * the two sets are identical. + * + * With SETUID_SCRIPTS_ARE_SECURE_NOW, could we use fdscript, also or + * instead, or would that be "too late"? (We never have suidscript, can + * we be sure to have fdscript?) + * + * Catch things with suidscript (in descendant of suidperl), even with + * right UID/GID. Was already checked in suidperl, with #ifdef IAMSUID, + * below; but I am paranoid. + * + * Also see comments about root running a setuid script, elsewhere. + */ + if (PL_suidscript >= 0) + Perl_croak(aTHX_ "No %s allowed with (suid) fdscript", s); +#ifdef IAMSUID + /* PSz 11 Nov 03 Catch it in suidperl, always! */ + Perl_croak(aTHX_ "No %s allowed in suidperl", s); +#endif /* IAMSUID */ } void @@ -4254,6 +4254,19 @@ int flock(int fd, int op); # define PERL_MOUNT_NOSUID M_NOSUID #endif +#if !defined(PERL_MOUNT_NOEXEC) && defined(MOUNT_NOEXEC) +# define PERL_MOUNT_NOEXEC MOUNT_NOEXEC +#endif +#if !defined(PERL_MOUNT_NOEXEC) && defined(MNT_NOEXEC) +# define PERL_MOUNT_NOEXEC MNT_NOEXEC +#endif +#if !defined(PERL_MOUNT_NOEXEC) && defined(MS_NOEXEC) +# define PERL_MOUNT_NOEXEC MS_NOEXEC +#endif +#if !defined(PERL_MOUNT_NOEXEC) && defined(M_NOEXEC) +# define PERL_MOUNT_NOEXEC M_NOEXEC +#endif + #endif /* IAMSUID */ #ifdef I_LIBUTIL @@ -250,6 +250,8 @@ END_EXTERN_C #define PL_expect (*Perl_Iexpect_ptr(aTHX)) #undef PL_fdpid #define PL_fdpid (*Perl_Ifdpid_ptr(aTHX)) +#undef PL_fdscript +#define PL_fdscript (*Perl_Ifdscript_ptr(aTHX)) #undef PL_filemode #define PL_filemode (*Perl_Ifilemode_ptr(aTHX)) #undef PL_forkprocess @@ -544,6 +546,8 @@ END_EXTERN_C #define PL_subline (*Perl_Isubline_ptr(aTHX)) #undef PL_subname #define PL_subname (*Perl_Isubname_ptr(aTHX)) +#undef PL_suidscript +#define PL_suidscript (*Perl_Isuidscript_ptr(aTHX)) #undef PL_sv_arenaroot #define PL_sv_arenaroot (*Perl_Isv_arenaroot_ptr(aTHX)) #undef PL_sv_count |