From: Tomas Doran <bobtfish@bobtfish.net>
Date: Wed, 1 Sep 2010 21:52:43 +0000 (+0100)
Subject: Security patch for the Data::Dumper serializer
X-Git-Tag: 0.86~2
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=0fd45d2a3fbf41c788823c527f4c325cf6368c7c;hp=a34f2309c507d46bd490c85f13dd7d70478e6d99;p=catagits%2FCatalyst-Action-REST.git

Security patch for the Data::Dumper serializer

http://lists.scsys.co.uk/pipermail/catalyst-dev/2010-September/001829.html
---

diff --git a/Changes b/Changes
index 3147b95..fab2f3f 100644
--- a/Changes
+++ b/Changes
@@ -1,3 +1,6 @@
+
+  Make Data::Dumper unserializer safer by using a Safe compartment (Ton Voon)
+
 Thu 13 May 2010 10:09:19 CEST - Release 0.85
 
   Make Catalyst::Action::Serialize::View return directly rather than serializing
diff --git a/lib/Catalyst/Action/Deserialize/Data/Serializer.pm b/lib/Catalyst/Action/Deserialize/Data/Serializer.pm
index 26beb1d..e19d460 100644
--- a/lib/Catalyst/Action/Deserialize/Data/Serializer.pm
+++ b/lib/Catalyst/Action/Deserialize/Data/Serializer.pm
@@ -5,6 +5,9 @@ use namespace::autoclean;
 
 extends 'Catalyst::Action';
 use Data::Serializer;
+use Safe;
+my $compartment = Safe->new;
+$compartment->permit_only( qw(padany null lineseq const pushmark list anonhash anonlist refgen leaveeval undef) );
 
 our $VERSION = '0.85';
 $VERSION = eval $VERSION;
@@ -34,11 +37,18 @@ sub execute {
             }
             close(BODY);
         }
-        my $dso = Data::Serializer->new( serializer => $serializer );
         my $rdata;
-        eval {
-            $rdata = $dso->raw_deserialize($rbody);
-        };
+        if ( $serializer eq "Data::Dumper" ) {
+            # Taken from Data::Serialize::Data::Dumper::deserialize, but run within a Safe compartment
+            my $code = $rbody =~ /^\{/ ? "+".$rbody : $rbody;
+            $rdata = $compartment->reval( $code );
+        }
+        else {
+            my $dso = Data::Serializer->new( serializer => $serializer );
+            eval {
+                $rdata = $dso->raw_deserialize($rbody);
+            };
+        }
         if ($@) {
             return $@;
         }
diff --git a/t/data-serializer.t b/t/data-serializer.t
index b5fff06..77db8cf 100644
--- a/t/data-serializer.t
+++ b/t/data-serializer.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use Test::More tests => 29;
+use Test::More tests => 31;
 use FindBin;
 
 use lib ( "$FindBin::Bin/lib", "$FindBin::Bin/../lib" );
@@ -59,4 +59,22 @@ foreach my $content_type (keys(%ctypes)) {
     }
 }
 
+{
+        my $t = Test::Rest->new( 'content_type' => 'text/x-data-dumper' );
+
+        my $post_data = "{ 'sushi' => die('hack attempt') }";
+        my $mres_post = request(
+            $t->post(
+                url  => '/monkey_put',
+                data => $post_data,
+            )
+        );
+        ok( ! $mres_post->is_success, "POST Data::Dumper fails due to invalid input" );
+        like(
+            $mres_post->content,
+            qr%Content-Type text/x-data-dumper had a problem with your request.*'die' trapped by operation mask%s,
+            "POST Data::Dumper data error matches"
+        );
+}
+
 1;