Safe reval bleeds local variable values #6735

p5pRT · 2003-08-30T03:37:12Z

Migrated from rt.perl.org#23656 (status was 'resolved')

Searchable as RT23656$

p5pRT · 2003-08-30T03:37:12Z

From guy@albertelli.com

Created by guy@albertelli.com

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.

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);
}

Perl Info


Flags:
    category=library
    severity=low

Site configuration information for perl v5.8.1:

Configured by bhcompile at Wed Aug 20 09:13:59 EDT 2003.

Summary of my perl5 (revision 5.0 version 8 subversion 1) configuration:
  Platform:
    osname=linux, osvers=2.4.21-1.1931.2.393.entsmp, archname=i386-linux-thread-multi
    uname='linux daffy.perf.redhat.com 2.4.21-1.1931.2.393.entsmp #1 smp wed aug 13 21:51:41 edt 2003 i686 i686 i386 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -march=i386 -mcpu=i686 -Dversion=5.8.1 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dinstallprefix=/usr -Dprefix=/usr -Darchname=i386-linux -Dvendorprefix=/usr -Dsiteprefix=/usr -Dotherlibdirs=/usr/lib/perl5/5.8.1 -Duseshrplib -Dusethreads -Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2 -g -pipe -march=i386 -mcpu=i686',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='3.3.1 20030811 (Red Hat Linux 3.3.1-1)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.3.2'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic -Wl,-rpath,/usr/lib/perl5/5.8.1/i386-linux-thread-multi/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    RC4


@INC for perl v5.8.1:
    /usr/lib/perl5/5.8.1/i386-linux-thread-multi
    /usr/lib/perl5/5.8.1
    /usr/lib/perl5/site_perl/5.8.1/i386-linux-thread-multi
    /usr/lib/perl5/site_perl/5.8.1
    /usr/lib/perl5/site_perl/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/site_perl/5.8.0
    /usr/lib/perl5/site_perl
    /usr/lib/perl5/vendor_perl/5.8.1/i386-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.8.1
    /usr/lib/perl5/vendor_perl/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.8.0
    /usr/lib/perl5/vendor_perl
    /usr/lib/perl5/5.8.1/i386-linux-thread-multi
    /usr/lib/perl5/5.8.1
    .


Environment for perl v5.8.1:
    HOME=/home/albertel
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/kerberos/bin:/usr/local/bin:/usr/bin:/bin:/usr/X11R6/bin:/home/albertel/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

p5pRT · 2003-09-04T00:16:31Z

From ben.goldberg@hotpop.com

"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 \{ @&#8203;\_ = \(\); eval $expr; \}'\, $obj\->\{Root\}\);

    if \($strict\) \{ use strict; $Safe&#8203;::evalsub = eval $evalcode; \}
    else         \{  no strict; $Safe&#8203;::evalsub = eval $evalcode; \}
\}
return Opcode&#8203;::\_safe\_call\_sv\($\_\[0\]\->\{Root\}\, $\_\[0\]\->\{Mask\}\, $Safe&#8203;::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;}

p5pRT · 2003-09-04T08:57:36Z

From guy@albertelli.com

"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.

Dosen't seem to bleed default_root or default_share, and the bleed of
things other than expr is a perl 5.8.1 ism:

Test script:
use strict;
use Safe;
print $]."\n";
my $safe=new Safe;

foreach my $name ('default_root','default_share','expr','obj','strict','root','evalcode','evalsub') {
print("Test $name is :".$safe->reval('return $'.$name.';').":\n");
}

5.008001
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is :Safe=HASH(0x9f2bd28):
Test strict is ::
Test root is :Safe::Root0:
Test evalcode is :package Safe::Root0; sub { @_ = (); eval $expr; }:
Test evalsub is :CODE(0x9f45d0c):

5.008
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is ::
Test strict is ::
Test root is ::
Test evalcode is ::
Test evalsub is ::

5.006001
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is ::
Test strict is ::
Test root is ::
Test evalcode is ::
Test evalsub is ::

In theory I agree, but in practice it doesn't look to be an issue.

(And what changed in 5.8.1 to make this "break" more?)

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 \{ @&#8203;\_ = \($; eval $expr; \}'\, $obj\->\{Root\}\);

    if $$strict$ \{ use strict; $Safe&#8203;::evalsub = eval $evalcode; \}
    else         \{  no strict; $Safe&#8203;::evalsub = eval $evalcode; \}
\}
return Opcode&#8203;::\_safe\_call\_sv$$\_\[0\]\->\{Root\}\, $\_\[0\]\->\{Mask\}\, $Safe&#8203;::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.

I understand why you say that, but it does succeed as $default_root,
$default_share are unviewable.

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);

I tried this, I doesn't seem to get it to work.

I put the above subs in Safe.pm right about the default_root
declaration, and set reval to :

sub reval {
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; }', $root);
my $evaler = $strict ? \&_eval_no_lexicals_strict :
\&_eval_no_lexicals_nostrict;
my $evalsub = $evaler->($evalcode);

return Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub);
}

With this test script:

use strict;
use lib '/home/httpd/lib/perl';
use Safe;
print $]."\n";
my $safe=new Safe;

print("Test a is :".$safe->reval('$a=1;return $a;').":\n");
print(" Eval errors :$@:\n");

I get:

5.008001
Test a is ::
Eval errors ::

I can't get it to generate any warning or errors.

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.

I'll have to take your word for it.

--
guy@albertelli.com LON-CAPA Developer 0-7-6-1-

p5pRT · 2003-09-04T11:02:10Z

From @iabyn

On Thu, Sep 04, 2003 at 01:32:57AM -0400, Guy Albertelli II wrote:

Dosen't seem to bleed default_root or default_share, and the bleed of
things other than expr is a perl 5.8.1 ism:

Test script:
use strict;
use Safe;
print $]."\n";
my $safe=new Safe;

foreach my $name ('default_root','default_share','expr','obj','strict','root','evalcode','evalsub') {
print("Test $name is :".$safe->reval('return $'.$name.';').":\n");
}

5.008001
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is :Safe=HASH(0x9f2bd28):
Test strict is ::
Test root is :Safe::Root0:
Test evalcode is :package Safe::Root0; sub { @_ = (); eval $expr; }:
Test evalsub is :CODE(0x9f45d0c):

5.008
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is ::
Test strict is ::
Test root is ::
Test evalcode is ::
Test evalsub is ::

5.006001
Test default_root is ::
Test default_share is ::
Test expr is :return $expr;:
Test obj is ::
Test strict is ::
Test root is ::
Test evalcode is ::
Test evalsub is ::

This is because 5.8.1 includes fixes that allow nested evals to still see
their full lexical scope.

As of 5.8.1, to code to be eval'ed will be able to see:

* any lexical vars declared above it but within reval();
* any lexical vars declared above reval(), but which are referred to
within reval() - ie reval() acting as a closure. Currently there are no
such variables.
Any other lexicals will have gone out of scope by then, and any attempt to
use them within an eval will give a '$foo is not available' warning.

Having said that, I think Benjamin's suggestion of having an eval function
at the very start of the script before any lexicals of any description
have been declared, is a sensible idea. It will help protect us against
any future changes to the code that inadvertently create a closure.

Dave.

--
"Foul and greedy Dwarf - you have eaten the last candle."
- "Hoardes of the Things", BBC Radio.

p5pRT · 2003-09-04T23:23:07Z

From ben.goldberg@hotpop.com

"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 \{ @&#8203;\_ = \(\); eval $expr; \}'\, $obj\->\{Root\}\);

    if \($strict\) \{ use strict; $Safe&#8203;::evalsub = eval $evalcode; \}
    else         \{  no strict; $Safe&#8203;::evalsub = eval $evalcode; \}
\}
return Opcode&#8203;::\_safe\_call\_sv\($\_\[0\]\->\{Root\}\, $\_\[0\]\->\{Mask\}\, $Safe&#8203;::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;}

p5pRT · 2003-09-04T23:36:52Z

From guy@albertelli.com

This is because 5.8.1 includes fixes that allow nested evals to still see
their full lexical scope.

Thanks, I hadn't noticed that change.

Having said that, I think Benjamin's suggestion of having an eval function
at the very start of the script before any lexicals of any description
have been declared, is a sensible idea. It will help protect us against
any future changes to the code that inadvertently create a closure.

I agree completely, and in fact it was the first thing I tried.

But I have been unable to get it to work at all.

Once I implement the suggested change all reval's silently fail, and I
don't have enough experience trying to debug perl to be able to tell
why they silently fail.

--
guy@albertelli.com LON-CAPA Developer 0-7-6-1-

p5pRT · 2003-09-04T23:37:23Z

From guy@albertelli.com

Having said that, I think Benjamin's suggestion of having an eval function
at the very start of the script before any lexicals of any description
have been declared, is a sensible idea. It will help protect us against
any future changes to the code that inadvertently create a closure.

Okay, so I think I know why it continues to fail.

my $evalcode = sprintf('package %s; sub { @_ = (); eval $expr; }',$root);

This line depends upon the fact that the $expr is in scope when we do
the eval.

The naive:
my $evalcode = sprintf('package %s; sub { @_ = (); eval %s; }',$root,$expr);

with test code
print("Test a is :".$safe->reval('$a=1;').":\n");

Throws errors:
Can't modify eval "string" in scalar assignment at (eval 1) line 1, at EOF
(in cleanup) Undefined subroutine &main:: called at
/home/httpd/lib/perl/Safe.pm line 226.

So I guess I am out of my depth.

--
guy@albertelli.com LON-CAPA Developer 0-7-6-1-

p5pRT · 2003-09-07T18:15:38Z

From @iabyn

Okay, here's a patch. When Safe code is eval'ed now, there are no
lexical variables declared within any nested scope, apart from one called
$__ExPr__, which is actually local to the code being evaled and has no
connection with a lexical variable of the same name that happens to be in
an outer nested scope and which is used by Safe.pm.

So its secure, but ever so slightly untidy.

The one remaining lexical is needed because a closure is used to pass the
code to be eval'ed. This is necessary because Opcode.xs provides a
safe_call_cv() function but not a safe_eval_pv()-type function.

So Safe.pm used to do the rough equivalent of

sub reval {
my $expr = (code to be evaled);
safe_call_sv(sub { eval $expr });
);
My patch moves the relevant code to the start of the file to avoid most
lexical variables being in scope; renames $expr to $__ExPr__, and adds an
extra my to the start of the code:

sub { eval 'my $__ExPr__;' . $__ExPr__ };

Given that this ia security patch, other eyes may want to give it
a one-over.

Dave.

--
Justice is when you get what you deserve.
Law is when you get what you pay for.

Inline Patch

--- ext/Opcode/Safe.pm-	Sun Sep  7 17:24:51 2003
+++ ext/Opcode/Safe.pm	Sun Sep  7 18:58:44 2003
@@ -5,6 +5,26 @@
 
 $Safe::VERSION = "2.09";
 
+# *** Don't declare any lexicals above this point ***
+#
+# This function should return a closure which contains an eval that can't
+# see any lexicals in scope (apart from __ExPr__ which is unavoidable)
+
+sub lexless_anon_sub {
+		 # $_[0] is package;
+		 # $_[1] is strict flag;
+    my $__ExPr__ = $_[2];   # must be a lexical to create the closure that
+			    # can be used to pass the value into the safe
+			    # world
+
+    # 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)
+    eval sprintf
+    'package %s; %s strict; sub { @_=(); eval q[my $__ExPr__;] . $__ExPr__; }',
+		$_[0], $_[1] ? 'use' : 'no';
+}
+
 use Carp;
 
 use Opcode 1.01, qw(
@@ -211,15 +231,7 @@
     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; }', $root);
-    my $evalsub;
-
-    if ($strict) { use strict; $evalsub = eval $evalcode; }
-    else         {  no strict; $evalsub = eval $evalcode; }
-
+    my $evalsub = lexless_anon_sub($root,$strict, $expr);
     return Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub);
 }

p5pRT · 2003-09-07T19:42:31Z

From @rgs

Dave Mitchell wrote:

Okay, here's a patch. When Safe code is eval'ed now, there are no
lexical variables declared within any nested scope, apart from one called
$__ExPr__, which is actually local to the code being evaled and has no
connection with a lexical variable of the same name that happens to be in
an outer nested scope and which is used by Safe.pm.

Thanks, applied as #21063. (I also incremented Safe's VERSION.)

p5pRT · 2003-09-07T19:45:43Z

@rgs - Status changed from 'new' to 'resolved'

p5pRT · 2003-09-12T14:48:17Z

From mjtg@cam.ac.uk

Dave Mitchell <davem@fdgroup.com> wrote

Okay, here's a patch. When Safe code is eval'ed now, there are no
lexical variables declared within any nested scope, apart from one called
$__ExPr__, which is actually local to the code being evaled and has no
connection with a lexical variable of the same name that happens to be in
an outer nested scope and which is used by Safe.pm.

No need for an extra lexical - you've got the eval() and the sub {}
the wrong way out.

So Safe.pm used to do the rough equivalent of
sub reval \{
 my $expr = $code to be evaled$;
 safe\_call\_sv$sub \{ eval $expr \}$;
\);

That should be written

sub reval {
safe_call_sv(eval 'sub {' . shift() . '}');
};

(with of course error checking etc).

Indeed, you only need do the eval() in the subroutine at the top of the code
- the safe_call_cv() can be done anywhere you like.

Mike Guy

p5pRT · 2003-09-12T20:25:46Z

From @iabyn

On Fri, Sep 12, 2003 at 03:47:31PM +0100, Mike Guy wrote:

Dave Mitchell <davem@fdgroup.com> wrote

Okay, here's a patch. When Safe code is eval'ed now, there are no
lexical variables declared within any nested scope, apart from one called
$__ExPr__, which is actually local to the code being evaled and has no
connection with a lexical variable of the same name that happens to be in
an outer nested scope and which is used by Safe.pm.

No need for an extra lexical - you've got the eval() and the sub {}
the wrong way out.
So Safe.pm used to do the rough equivalent of
sub reval \{
 my $expr = $code to be evaled$;
 safe\_call\_sv$sub \{ eval $expr \}$;
\);
That should be written
  sub reval \{
  safe\_call\_sv$eval 'sub \{' \. shift\($ \. '\}'\);
  \};

No, because then the eval is executed outside the sandbox,

eg

reval(' BEGIN { system "rm -rf /" }' );

or

reval( '1 } system "rm -rf /"; { 1' );

--
Thank God I'm an atheist.....

p5pRT · 2004-02-26T14:59:09Z

@iabyn - Status changed from 'open' to 'resolved'

p5pRT closed this as completed Feb 26, 2004

p5pRT added Severity Low distro-Linux type-library labels Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Safe reval bleeds local variable values #6735

Safe reval bleeds local variable values #6735

p5pRT commented Aug 30, 2003

p5pRT commented Aug 30, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 4, 2003

p5pRT commented Sep 7, 2003

p5pRT commented Sep 7, 2003

p5pRT commented Sep 7, 2003

p5pRT commented Sep 12, 2003

p5pRT commented Sep 12, 2003

p5pRT commented Feb 26, 2004

Safe reval bleeds local variable values #6735

Safe reval bleeds local variable values #6735

Comments

p5pRT commented Aug 30, 2003

p5pRT commented Aug 30, 2003

From guy@albertelli.com

Created by guy@albertelli.com

p5pRT commented Sep 4, 2003

From ben.goldberg@hotpop.com

p5pRT commented Sep 4, 2003

From guy@albertelli.com

p5pRT commented Sep 4, 2003

From @iabyn

p5pRT commented Sep 4, 2003

From ben.goldberg@hotpop.com

p5pRT commented Sep 4, 2003

From guy@albertelli.com

p5pRT commented Sep 4, 2003

From guy@albertelli.com

p5pRT commented Sep 7, 2003

From @iabyn

p5pRT commented Sep 7, 2003

From @rgs

p5pRT commented Sep 7, 2003

p5pRT commented Sep 12, 2003

From mjtg@cam.ac.uk

p5pRT commented Sep 12, 2003

From @iabyn

p5pRT commented Feb 26, 2004