Item15192: use a Safe for $EVAL()

foswiki · May 23, 2023 · 0714f19 · 0714f19
1 parent a0bc7a1
commit 0714f19
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 26 deletions.
diff --git a/SpreadSheetPlugin/lib/Foswiki/Plugins/SpreadSheetPlugin/Calc.pm b/SpreadSheetPlugin/lib/Foswiki/Plugins/SpreadSheetPlugin/Calc.pm
@@ -13,6 +13,7 @@ use warnings;
 use HTML::Entities;
 use Time::Local;
 use Time::Local qw( timegm_nocheck timelocal_nocheck );    # Necessary for DOY
+use Safe ();
 
 # =========================
 my $web;
@@ -29,6 +30,7 @@ my $escCloseP   = "\3";
 my $escNewLn    = "\4";
 my %varStore    = ();
 my $dontSpaceRE = "";
+my $safeCpt;
 
 # SMELL: I18N
 my @monArr = (
@@ -1722,8 +1724,14 @@ sub _getNumber {
 # =========================
 sub _safeEvalPerl {
     my ($theText) = @_;
+
     $theText = '' unless defined $theText;
 
+    unless ( defined $safeCpt ) {
+        $safeCpt = Safe->new();
+        $safeCpt->deny(":subprocess");
+    }
+
     # Allow only simple math with operators - + * / % ( )
     $theText =~ s/\%\s*[^\-\+\*\/0-9\.\(\)]+//g; # defuse %hash but keep modulus
 
@@ -1740,16 +1748,10 @@ sub _safeEvalPerl {
     $theText =~ /(.*)/;
     $theText = $1;
 
-    # disable glob for security reasons
-    while ( $theText =~ s/\<[\.\*\/\?\se\<]*\>/ /g ) {
-        1;
-    }
-
     return "" unless defined($theText);
 
-    local $SIG{__DIE__} =
-      sub { Foswiki::Func::writeDebug( $_[0] ); warn $_[0] };
-    my $result = eval $theText;
+    local $SIG{__DIE__} = sub { warn $_[0] };
+    my $result = $safeCpt->reval($theText);
 
     if ($@) {
         $result = $@;
@@ -1764,6 +1766,7 @@ sub _safeEvalPerl {
     else {
         $result = 0 unless ($result);    # logical false is "0"
     }
+
     return $result;
 }
 

diff --git a/SpreadSheetPlugin/test/unit/SpreadSheetPlugin/SpreadSheetPluginTests.pm b/SpreadSheetPlugin/test/unit/SpreadSheetPlugin/SpreadSheetPluginTests.pm
@@ -438,24 +438,12 @@ sub test_EVAL_GLOB {
     my ($this) = @_;
 
     $this->assert( $this->CALC('$EVAL(1 < 2 + 2 > 1)') == 1 );
-    $this->assert( $this->CALC('$EVAL(1 <2 <> )') == 1 );
-    $this->assert( $this->CALC('$EVAL(<>)') == 0 );
-    $this->assert( $this->CALC('$EVAL(<<>>)')   =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(<<<>>>)') =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(<*>)') == 0 );
-    $this->assert( $this->CALC('$EVAL((<*>))') == 0 );
-    $this->assert( $this->CALC('$EVAL(< * >)') == 0 );
-    $this->assert( $this->CALC('$EVAL(<../../../ee*/* >)') == 0 );
-    $this->assert( $this->CALC('$EVAL(2+<>+2)') == 4 );
-    $this->assert( $this->CALC('$EVAL(2+<   >+2)') == 4 );
-    $this->assert( $this->CALC('$EVAL(%+.<*>.2)') =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(2+<e>+2)') == 4 );
-    $this->assert( $this->CALC('$EVAL(2+<x>+2)') == 4 );
-    $this->assert( $this->CALC('$EVAL(%-.<*>.2)')    =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(%+.<../*>.2)') =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(3-<../*>-3)') == 6 );
-    $this->assert( $this->CALC('$EVAL(%-.<ee*/..>.%-)') =~ /^ERROR:/ );
-    $this->assert( $this->CALC('$EVAL(%-.<<>../*>.%-)') =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL(1 <2 <> )') =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL(<>)')       =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL(<<>>)')     =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL(<<<>>>)')   =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL(<*>)')      =~ /^ERROR:/ );
+    $this->assert( $this->CALC('$EVAL((<*>))')    =~ /^ERROR:/ );
 }
 
 sub test_EVEN {