Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

On Tue, Feb 23, 2010 at 02:59:05PM -0700, Alex Hunsaker wrote:
> On Tue, Feb 23, 2010 at 14:29, Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> wrote:
> > David Wheeler has discovered a new PL/Perl failure when using Safe 2.2x.
>
> *sigh*.
>
> Thanks for all the testing David! And of course many thanks to Tim
> for all the hours of analysis and hard work!
>
> > That'll fix the immediate problem for all PostgreSQL versions.
> > Security will be no better or worse that it was before Safe 2.20.
> > The old sort { } bug (where $a & $b) don't work will return but
> > that seems a very small price to pay for a simple fix.
>
> +1. Although I think I might be in favor of just ripping it out all
> together. There are a couple of goofy things about the current
> behavior anyway. Notably if you are not using a threaded build you
> never get the extra protection.

Yes, that needs resolving. Thanks for the reminder.

> At the time it was really just a fix
> for the namespace issues. I do agree the increased security is
> nice... But that was not the primary goal :)

We'll discuss the issues and API option for Safe on the perl5-porters
mailing list.

> > I'd like to see PostgreSQL re-enable use of the wrapped subroutines
> > in the future but it'll require some development effort.
>
> I would to.
>
> > The plperl
> > entry points will need to detect if Safe is 'in effect' and locally undo
> > it.  There's some prior art in http://search.cpan.org/perldoc?Safe::Hole
> > that might be useful.
>
> Yick... There must be a better way?

Doesn't seem too icky. Basically plperl would need to save the values of
PL_defstash, PL_incgv and PL_op_mask when a plperl interpreter is
initialized. And then local()'ly restore them in the plperl entry points.
Should only be a few lines of code - in theory :)

Tim.

On Tue, Feb 23, 2010 at 15:54, Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> wrote:
> Doesn't seem too icky. Basically plperl would need to save the values of
> PL_defstash, PL_incgv and PL_op_mask when a plperl interpreter is
> initialized.  And then local()'ly restore them in the plperl entry points.
> Should only be a few lines of code - in theory :)

Ok I can get behind this. I did some testing and we could probably
even store less than than that if we could do the equivalent of:
Safe->share('::mksafefunc');
pl_perl_createsub()
Safe->unshare('::mksafefunc');

See my quick test case:
my $s = Safe->new();
$s->permit(qw(print));

our $obj = sub { return eval 'sub { print "b\n";}' };
$obj->()->();
$s->share(qw($obj));
$s->reval('$obj->()->();');
print $@ . "\n";
---
b
b

(BTW the above fails with the helpful "Undefined subroutine &main::
called at (eval 6) line 1." without the ->permt(qw(print))")

So we might not even have to store anything if we can make it behave
as above. However I think it will be cleaner to me to locally restore
them as your originally suggested.

BTW sorry for my scatter braininess. I keep flip flopping between
revet Safe or patch postgres. ATM it seems if the patch is simple we
can get it back patched and into 9.0. So my vote is lets try that, if
its to hard then lets see about reverting Safe. Sound Ok?

On Wed, Feb 24, 2010 at 22:01, Alex Hunsaker <badalex(at)gmail(dot)com> wrote:
> Ok I can get behind this.  I did some testing and we could probably
> even store less than than that if we could do the equivalent of:
> Safe->share('::mksafefunc');
> pl_perl_createsub()
> Safe->unshare('::mksafefunc');

On 2nd thought this basically requires your fix anyway. To make it so
you can share something in safe from within safe means we will need to
enable more opcodes there... so it would end up being the same
solution.