Re: plperl Safe restrictions - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: plperl Safe restrictions
Date
Msg-id 416FEC68.6000909@dunslane.net
Whole thread Raw
In response to Re: plperl Safe restrictions  (David Helgason <david@uti.is>)
Responses Re: [PATCHES] plperl Safe restrictions
Re: [PATCHES] plperl Safe restrictions
List pgsql-hackers

David Helgason wrote:

> On 14. okt 2004, at 21:09, Andrew Dunstan wrote:
>
>> It has just been brought to my attention that we are being very
>> restrictive about what we allow to be done in trusted plperl.
>> Basically we allow the :default and :base_math set of operations (run
>> perldoc Opcode or see
>> http://www.perldoc.com/perl5.8.0/lib/Opcode.html for details of what
>> these mean). In particular, we do not allow calls to perl's builtin
>> sort, which is unpleasant, and on reviewing the list it seems to me
>> we could quite reasonably allow access to pack and unpack also. bless
>> and sprintf are also likely candidates for inclusion - I have not
>> finished reviewing the list, and would welcome advice from perl gurus
>> on this.
>
>
> pack and unpack are unfortunately not safe. Very useful, but they
> allow write/read access to random memory. It's really a shame perl
> doesn't have a pragma to make them safe or have safe versions of them.
>
> Bless should be OK, unless sensitive objects are provided to the
> procedures.
>
> A postgres question I don't know the answer to is whether allowing the
> user to trigger a segfault is a security problem. If it isn't, several
> opcodes may probably be allowed, including sort and sprintf. If it is,
> well, you need only follow the perl5-porters list to know that there's
> banal perl structures are continuously being found that will segfault
> perl, some at compile time, other at runtime.


OK, based on this and some further thought, I have prepared the attached
patch which does the right thing, I think, both in terms of what we
allow and what we don't.

First, we tighten security by disallowing access to srand and IO
functions on existing filehandles (other IO ops are already disallowed).

The we relax the restrictions by allowing access to perl's sort, sprintf
and time routines. I decided against pack/unpack based on the above, and
also decided that I couldn't think of any case where bless would have
any practical use - although that might change later. I'm trying to keep
changes minimal here. I don't believe that "time" carries any
significant security implications, and I think the dangers from "sort"
and "sprintf" are not so great as to disallow them. They might cause a
SEGV in a pathological case, but that doesn't give the user access to
the machine - if they can login to postgres they can probably mount any
number of DOS attacks anyway.

To answer David's question, the man says this about trusted functions:
"the TRUSTED flag should only be given for languages that do not allow
access to database server internals or the file system".  I think the
changes I propose fit in with that statement.

The patch also does some other inconsequential tidying of overlong
lines, and removes some unnecessary ops in the unsafe case. These are
basically cosmetic - the only significant part is replacing this:

    $PLContainer->permit(':base_math');

with this:

    $PLContainer->permit(qw[:base_math !:base_io !srand sort sprintf time]);

I have tested and it appears to do the right thing, both for the things
excluded and those included.

cheers

andrew



Index: src/pl/plperl/plperl.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.54
diff -c -r1.54 plperl.c
*** src/pl/plperl/plperl.c    7 Oct 2004 19:01:09 -0000    1.54
--- src/pl/plperl/plperl.c    15 Oct 2004 14:48:18 -0000
***************
*** 250,266 ****

      static char *safe_ok =
      "use vars qw($PLContainer); $PLContainer = new Safe('PLPerl');"
!     "$PLContainer->permit_only(':default');$PLContainer->permit(':base_math');"
!     "$PLContainer->share(qw[&elog &spi_exec_query &DEBUG &LOG &INFO &NOTICE &WARNING &ERROR %SHARED ]);"
      "sub ::mksafefunc { return $PLContainer->reval(qq[sub { $_[0] $_[1]}]); }"
                 ;

      static char *safe_bad =
      "use vars qw($PLContainer); $PLContainer = new Safe('PLPerl');"
!     "$PLContainer->permit_only(':default');$PLContainer->permit(':base_math');"
!     "$PLContainer->share(qw[&elog &DEBUG &LOG &INFO &NOTICE &WARNING &ERROR %SHARED ]);"
      "sub ::mksafefunc { return $PLContainer->reval(qq[sub { "
!     "elog(ERROR,'trusted perl functions disabled - please upgrade perl Safe module to at least 2.09');}]); }"
                 ;

      SV           *res;
--- 250,269 ----

      static char *safe_ok =
      "use vars qw($PLContainer); $PLContainer = new Safe('PLPerl');"
!     "$PLContainer->permit_only(':default');"
!     "$PLContainer->permit(qw[:base_math !:base_io !srand sort sprintf time]);"
!     "$PLContainer->share(qw[&elog &spi_exec_query &DEBUG &LOG "
!     "&INFO &NOTICE &WARNING &ERROR %SHARED ]);"
      "sub ::mksafefunc { return $PLContainer->reval(qq[sub { $_[0] $_[1]}]); }"
                 ;

      static char *safe_bad =
      "use vars qw($PLContainer); $PLContainer = new Safe('PLPerl');"
!     "$PLContainer->permit_only(':default');"
!     "$PLContainer->share(qw[&elog &ERROR ]);"
      "sub ::mksafefunc { return $PLContainer->reval(qq[sub { "
!     "elog(ERROR,'trusted perl functions disabled - "
!     "please upgrade perl Safe module to at least 2.09');}]); }"
                 ;

      SV           *res;

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: plans for bitmap indexes?
Next
From: Bruce Momjian
Date:
Subject: Re: Strange code in initdb