Front page | perl.perl5.porters |
Postings from January 2004
Re: suidperl security
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
January 16, 2004 16:44
Subject:
Re: suidperl security
Message ID:
20040117004415.GC12426@plum.flirble.org
Sorry about letting this slip. I don't use suidperl, so it something that
burns in my mind, and I'm not aware of anyone active on the list who uses
it, or even expresses interest in it.
On Thu, Dec 11, 2003 at 11:31:15AM +1100, Paul Szabo wrote:
> >>>>> ... you'd had to use a static variable ...
> >>>> That is a "red herring". Used a static variable instead of passing it as
> >>>> argument to various functions, only to avoid re-writing their prototypes.
> >
> > Having checked over the patch again (just for this) am I correct in thinking
> > that there are only two prototypes involved, and both were static functions?
>
> I guess you noticed open_script and validate_suid. I made fdscript global
> so it could be used in (would be seen by) forbid_setid; forbid_setid is
> called also from routines where fdscript was not visible (other than
> parse_body or open_script): moreswitches and find_beginning.
I've checked the execution route through script open.
I don't think that fdscript can safely be a global. It needs to be thread
local storage, else we create a new race condition in any third party
application that embeds perl interpreters and multithreads.
The example code given in perlembed.pod shows
int main(int argc, char **argv, char **env)
{
PERL_SYS_INIT3(&argc,&argv,&env);
my_perl = perl_alloc();
perl_construct(my_perl);
PL_exit_flags |= PERL_EXIT_DESTRUCT_END;
perl_parse(my_perl, NULL, argc, argv, (char **)NULL);
perl_run(my_perl);
perl_destruct(my_perl);
perl_free(my_perl);
PERL_SYS_TERM();
}
The call chain is
perl_parse
[optionally S_vparse_body]
S_parse_body
which assigns -1 to fdscript
With fdscript as a local variable in S_parse_body, two perl interpreter
threads can be started simultaneously without problems. If this variable
becomes a global then we have a potential thread race.
I'm not sure how multithreaded mod_perl works. It's also conceivable that
someone has written a multi-threading application similar in functionality
to PPerl ( http://search.cpan.org/~msergeant/PPerl-0.24/ ).
I think that it's quite safe to change the prototypes of genuine static
functions within stable releases - hence changing forbid_setid from
STATIC void
S_forbid_setid(pTHX_ char *s)
to
STATIC void
S_forbid_setid(pTHX_ char *s, int fdscript)
would allow fdscript to stay as a local variable (and hence one per thread)
except that fdscript is also called from Perl_morewitches, which has no
access to fdscript. This rules out that change. Instead it should all
work with fdscript added to thread local storage.
However, I can show that the addition in S_forbid_setid:
if (fdscript >= 0)
Perl_croak(aTHX_ "No %s allowed with fdscript", s);
causes a change in behaviour for existing non-setid scripts.
As is with bash on FreeBSD:
perl <( echo '#!perl -DA'; echo 'print "$0\n"')
/dev/fd/63
With your patch with bash on FreeBSD:
perl <( echo '#!perl -DA'; echo 'print "$0\n"')
No -D allowed with fdscript at /dev/fd/63 line 1.
I doubt that people generate perl scripts with echo inside subshells, but
this is a change in behaviour. Is there a viable way of achieving the
security needed for the setid case by adding an independent flag in the
if (*s) {
scriptname = savepv(s + 1);
Safefree(PL_origfilename);
PL_origfilename = scriptname;
}
section of S_open_script, set when the script name has been passed in
the style:
/dev/fd/###/script/name/goes/here
(again this flag would have to be thread local rather than global)
Nicholas Clark
Thread Previous
|
Thread Next