"guy@albertelli.com (via RT)" wrote:
[snip]
> The reval method of a Safe object bleeds the local variables into
> the expression being evaled.
> 
> Example:
> ----
> #!/usr/bin/perl
> use Safe;
> $safe=new Safe;
> print("Test 1 is ".$safe->reval('return $expr;')."\n");
> print("Test 2 is" .$safe->reval('return $expe;')."\n");
> ----
> 
> Incorrectly prints:
> Test 1 is return $expr;
> Test 2 is
> 
> Rather than the correct:
> Test 1 is
> Test 2 is
> 
> This occurs for all of the local variables in reval.

Not just those, but *all* lexicals that have been declared at that time.

Including $default_root and $default_share.  I can imagine someone
changing $default_share to ['*main::'], and then taking advantage of that
the next time a new safe is created.  Brr.

> I suggest modifying the reval Subroutine to be:
> 
> sub reval {
>     $Safe::evalsub;
>     {
>         my ($obj, $expr, $strict) = @_;
>         my $root = $obj->{Root};
> 
>         # Create anon sub ref in root of compartment.
>         # Uses a closure (on $expr) to pass in the code to be executed.
>         # (eval on one line to keep line numbers as expected by caller)
>         my $evalcode = sprintf('package %s; sub { @_ = (); eval $expr; }', $obj->{Root});
> 
>         if ($strict) { use strict; $Safe::evalsub = eval $evalcode; }
>         else         {  no strict; $Safe::evalsub = eval $evalcode; }
>     }
>     return Opcode::_safe_call_sv($_[0]->{Root}, $_[0]->{Mask}, $Safe::evalsub);
> }

I don't think that changing the $evalsub variable from a lexical to a
package variable will prevent the problem.  In fact, I don't see how it
could possibly do so.

A correct solution would be to perform the eval at a point in time when
there *are* no lexicals.

I would suggest that we have near the beginning of package Safe, before
any lexicals are declared, two subs, like:

   sub _eval_no_lexicals_strict  { use strict; eval shift }
   sub _eval_no_lexicals_nostrict { no strict; eval shift }

Then, later on, you'd have:

   my $evaler = $strict ? \&_eval_no_lexicals_strict :
                          \&_eval_no_lexicals_nostrict;
   my $evalsub = $evaler->($evalcode);

An even safer solution might be to get the code evaluated through perl's
do EXPR mechanism, to *guarantee* that it's given a fresh clean lexical
scope (and nothing in $^H or %^H to fiddle with).

This could be done with something like:

   unshift @INC, sub {
      shift @INC;
      return IO::Scalar->new(\$evalcode);
   };
   my $evalsub = do "(safeeval $n)";

Alas, this would probably be too much overhead... Especially when putting
two subs, _eval_no_lexicals_(no)?strict, near the top of Safe.pm, will
suffice for most purposes.  At least, it will until someone finds an
exploit for the _eval_no_lexicals things.

-- 
$a=24;split//,240513;s/\B/ => /for@@=qw(ac ab bc ba cb ca
);{push(@b,$a),($a-=6)^=1 for 2..$a/6x--$|;print "$@[$a%6
]\n";((6<=($a-=6))?$a+=$_[$a%6]-$a%6:($a=pop @b))&&redo;}