Re: Patch for Data::Dumper - CVE-2014-4330

2014-10-25 Thread Ingo Schwarze 
Hi Alexander,

Alexander Bluhm wrote on Fri, Oct 24, 2014 at 10:55:07PM +0200:
 On Fri, Oct 24, 2014 at 10:40:55PM +0200, Alexander Bluhm wrote:

 Here is the diff that applies to -current.  I have compared it with
 the perl git and with Data::Dumper on CPAN.  It looks correct.

Confirmed.

 I have forgotten to cvs add dist/Data-Dumper/t/recurse.t
 so here is the diff with the new test.
 
 ok?

Reads good.  Also checked that the test suite succeeds
and that mitigation is effective (on i386).

So *if* we decide to patch it, ok schwarze@ for this version of the patch.

It physically and logically conflicts with future Perl updates,
though (changes to the same lines; changing the parameter lists
of the same functions in different ways).  I think it would be
nice to hear how Andrew thinks such issues should be addressed
to minimize the pain during future Perl updates.

 Alternatively we could update Data::Dumper to 2.154.

I'd say answering that question is at least in part Andrew's call.
I'm not sure whether that makes the upcoming 1.20 update easier
or harder.

Yours,
  Ingo


 Index: gnu/usr.bin/perl/MANIFEST
 ===
 RCS file: /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/MANIFEST,v
 retrieving revision 1.29
 diff -u -p -u -p -r1.29 MANIFEST
 --- gnu/usr.bin/perl/MANIFEST 24 Mar 2014 15:05:12 -  1.29
 +++ gnu/usr.bin/perl/MANIFEST 24 Oct 2014 20:19:35 -
 @@ -3155,6 +3155,7 @@ dist/Data-Dumper/t/perl-74170.t Regressi
  dist/Data-Dumper/t/purity_deepcopy_maxdepth.tSee if three 
 Data::Dumper functions work
  dist/Data-Dumper/t/qr.t  See if Data::Dumper works with qr|/|
  dist/Data-Dumper/t/quotekeys.t   See if Data::Dumper::Quotekeys works
 +dist/Data-Dumper/t/recurse.t See if Data::Dumper::Maxrecurse works
  dist/Data-Dumper/t/seen.tSee if Data::Dumper::Seen works
  dist/Data-Dumper/t/sortkeys.tSee if Data::Dumper::Sortkeys works
  dist/Data-Dumper/t/sparseseen.t  See if Data::Dumper::Sparseseen works
 Index: gnu/usr.bin/perl/patchlevel.h
 ===
 RCS file: /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/patchlevel.h,v
 retrieving revision 1.34
 diff -u -p -u -p -r1.34 patchlevel.h
 --- gnu/usr.bin/perl/patchlevel.h 5 Sep 2014 06:53:07 -   1.34
 +++ gnu/usr.bin/perl/patchlevel.h 24 Oct 2014 20:25:05 -
 @@ -134,6 +134,7 @@ hunk.
  static const char * const local_patches[] = {
   NULL
   ,Update libnet to 1.27
 + ,CVE-2014-4330
  #ifdef PERL_GIT_UNCOMMITTED_CHANGES
   ,uncommitted-changes
  #endif
 Index: gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm
 ===
 RCS file: 
 /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm,v
 retrieving revision 1.1.1.3
 diff -u -p -u -p -r1.1.1.3 Dumper.pm
 --- gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm   24 Mar 2014 14:58:59 
 -  1.1.1.3
 +++ gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm   24 Oct 2014 20:19:35 
 -
 @@ -56,6 +56,7 @@ $Useperl= 0 unless defined $
  $Sortkeys   = 0 unless defined $Sortkeys;
  $Deparse= 0 unless defined $Deparse;
  $Sparseseen = 0 unless defined $Sparseseen;
 +$Maxrecurse = 1000  unless defined $Maxrecurse;
  
  #
  # expects an arrayref of values to be dumped.
 @@ -92,6 +93,7 @@ sub new {
  'bless'= $Bless,# keyword to use for bless
  #expdepth   = $Expdepth,   # cutoff depth for explicit dumping
  maxdepth   = $Maxdepth,   # depth beyond which we give up
 + maxrecurse = $Maxrecurse, # depth beyond which we abort
  useperl= $Useperl,# use the pure Perl implementation
  sortkeys   = $Sortkeys,   # flag or filter for sorting hash keys
  deparse= $Deparse,# use B::Deparse for coderefs
 @@ -351,6 +353,12 @@ sub _dump {
return qq['$val'];
  }
  
 +# avoid recursing infinitely [perl #122111]
 +if ($s-{maxrecurse}  0
 +and $s-{level} = $s-{maxrecurse}) {
 +die Recursion limit of $s-{maxrecurse} exceeded;
 +}
 +
  # we have a blessed ref
  my ($blesspad);
  if ($realpack and !$no_bless) {
 @@ -683,6 +691,11 @@ sub Maxdepth {
defined($v) ? (($s-{'maxdepth'} = $v), return $s) : $s-{'maxdepth'};
  }
  
 +sub Maxrecurse {
 +  my($s, $v) = @_;
 +  defined($v) ? (($s-{'maxrecurse'} = $v), return $s) : $s-{'maxrecurse'};
 +}
 +
  sub Useperl {
my($s, $v) = @_;
defined($v) ? (($s-{'useperl'} = $v), return $s) : $s-{'useperl'};
 @@ -1105,6 +1118,16 @@ we don't venture into a structure.  Has 
  CData::Dumper::Purity is set.  (Useful in debugger when we often don't
  want to see more than enough).  Default is 0, which means there is
  no maximum depth.
 +
 +=item *
 +
 +$Data::Dumper::Maxrecurse  Ior  $IOBJ-Maxrecurse(I[NEWVAL])
 +
 +Can be set to a positive integer that specifies

Re: Patch for Data::Dumper - CVE-2014-4330

2014-10-25 Thread Andrew Fresh 
Although I don't have time to look in great detail, it seems ok on my phone. It 
should not effect the update to 5.20 which looks pretty good (apart from vax). 

Hopefully being away from the computer this weekend will make my brain grok gdb 
and vax enough after work that I can get 5.20 moving forward.

I am happy with just patching and trust ingo's ok. 

I will be updating some mail filters as I somehow missed this patch on p5p.

On October 25, 2014 10:59:05 AM PDT, Ingo Schwarze schwa...@usta.de wrote:
Hi Alexander,

Alexander Bluhm wrote on Fri, Oct 24, 2014 at 10:55:07PM +0200:
 On Fri, Oct 24, 2014 at 10:40:55PM +0200, Alexander Bluhm wrote:

 Here is the diff that applies to -current.  I have compared it with
 the perl git and with Data::Dumper on CPAN.  It looks correct.

Confirmed.

 I have forgotten to cvs add dist/Data-Dumper/t/recurse.t
 so here is the diff with the new test.
 
 ok?

Reads good.  Also checked that the test suite succeeds
and that mitigation is effective (on i386).

So *if* we decide to patch it, ok schwarze@ for this version of the
patch.

It physically and logically conflicts with future Perl updates,
though (changes to the same lines; changing the parameter lists
of the same functions in different ways).  I think it would be
nice to hear how Andrew thinks such issues should be addressed
to minimize the pain during future Perl updates.

 Alternatively we could update Data::Dumper to 2.154.

I'd say answering that question is at least in part Andrew's call.
I'm not sure whether that makes the upcoming 1.20 update easier
or harder.

Yours,
  Ingo


 Index: gnu/usr.bin/perl/MANIFEST
 ===
 RCS file: /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/MANIFEST,v
 retrieving revision 1.29
 diff -u -p -u -p -r1.29 MANIFEST
 --- gnu/usr.bin/perl/MANIFEST24 Mar 2014 15:05:12 -  1.29
 +++ gnu/usr.bin/perl/MANIFEST24 Oct 2014 20:19:35 -
 @@ -3155,6 +3155,7 @@ dist/Data-Dumper/t/perl-74170.tRegressi
  dist/Data-Dumper/t/purity_deepcopy_maxdepth.t   See if three
Data::Dumper functions work
  dist/Data-Dumper/t/qr.t See if Data::Dumper works with qr|/|
  dist/Data-Dumper/t/quotekeys.t  See if Data::Dumper::Quotekeys works
 +dist/Data-Dumper/t/recurse.tSee if Data::Dumper::Maxrecurse works
  dist/Data-Dumper/t/seen.t   See if Data::Dumper::Seen works
  dist/Data-Dumper/t/sortkeys.t   See if Data::Dumper::Sortkeys works
  dist/Data-Dumper/t/sparseseen.t See if Data::Dumper::Sparseseen
works
 Index: gnu/usr.bin/perl/patchlevel.h
 ===
 RCS file:
/data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/patchlevel.h,v
 retrieving revision 1.34
 diff -u -p -u -p -r1.34 patchlevel.h
 --- gnu/usr.bin/perl/patchlevel.h5 Sep 2014 06:53:07 -   1.34
 +++ gnu/usr.bin/perl/patchlevel.h24 Oct 2014 20:25:05 -
 @@ -134,6 +134,7 @@ hunk.
  static const char * const local_patches[] = {
  NULL
  ,Update libnet to 1.27
 +,CVE-2014-4330
  #ifdef PERL_GIT_UNCOMMITTED_CHANGES
  ,uncommitted-changes
  #endif
 Index: gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm
 ===
 RCS file:
/data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm,v
 retrieving revision 1.1.1.3
 diff -u -p -u -p -r1.1.1.3 Dumper.pm
 --- gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm  24 Mar 2014 14:58:59
-  1.1.1.3
 +++ gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm  24 Oct 2014 20:19:35
-
 @@ -56,6 +56,7 @@ $Useperl= 0 unless defined $
  $Sortkeys   = 0 unless defined $Sortkeys;
  $Deparse= 0 unless defined $Deparse;
  $Sparseseen = 0 unless defined $Sparseseen;
 +$Maxrecurse = 1000  unless defined $Maxrecurse;
  
  #
  # expects an arrayref of values to be dumped.
 @@ -92,6 +93,7 @@ sub new {
  'bless'= $Bless,# keyword to use for bless
  #expdepth   = $Expdepth,   # cutoff depth for explicit
dumping
  maxdepth   = $Maxdepth,   # depth beyond which we give up
 +maxrecurse = $Maxrecurse, # depth beyond which we abort
  useperl= $Useperl,# use the pure Perl
implementation
  sortkeys   = $Sortkeys,   # flag or filter for sorting hash
keys
  deparse= $Deparse,# use B::Deparse for coderefs
 @@ -351,6 +353,12 @@ sub _dump {
return qq['$val'];
  }
  
 +# avoid recursing infinitely [perl #122111]
 +if ($s-{maxrecurse}  0
 +and $s-{level} = $s-{maxrecurse}) {
 +die Recursion limit of $s-{maxrecurse} exceeded;
 +}
 +
  # we have a blessed ref
  my ($blesspad);
  if ($realpack and !$no_bless) {
 @@ -683,6 +691,11 @@ sub Maxdepth {
defined($v) ? (($s-{'maxdepth'} = $v), return $s) :
$s-{'maxdepth'};
  }
  
 +sub Maxrecurse {
 +  my($s, $v) = @_;
 +  defined($v) ? (($s-{'maxrecurse'}

Re: Patch for Data::Dumper - CVE-2014-4330

2014-10-24 Thread Alexander Bluhm 
On Fri, Oct 24, 2014 at 10:40:55PM +0200, Alexander Bluhm wrote:
 Here is the diff that applies to -current.  I have compared it with
 the perl git and with Data::Dumper on CPAN.  It looks correct.

I have forgotten to cvs add dist/Data-Dumper/t/recurse.t
so here is the diff with the new test.

ok?

bluhm

Index: gnu/usr.bin/perl/MANIFEST
===
RCS file: /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/MANIFEST,v
retrieving revision 1.29
diff -u -p -u -p -r1.29 MANIFEST
--- gnu/usr.bin/perl/MANIFEST   24 Mar 2014 15:05:12 -  1.29
+++ gnu/usr.bin/perl/MANIFEST   24 Oct 2014 20:19:35 -
@@ -3155,6 +3155,7 @@ dist/Data-Dumper/t/perl-74170.t   Regressi
 dist/Data-Dumper/t/purity_deepcopy_maxdepth.t  See if three Data::Dumper 
functions work
 dist/Data-Dumper/t/qr.tSee if Data::Dumper works with qr|/|
 dist/Data-Dumper/t/quotekeys.t See if Data::Dumper::Quotekeys works
+dist/Data-Dumper/t/recurse.t   See if Data::Dumper::Maxrecurse works
 dist/Data-Dumper/t/seen.t  See if Data::Dumper::Seen works
 dist/Data-Dumper/t/sortkeys.t  See if Data::Dumper::Sortkeys works
 dist/Data-Dumper/t/sparseseen.tSee if Data::Dumper::Sparseseen works
Index: gnu/usr.bin/perl/patchlevel.h
===
RCS file: /data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/patchlevel.h,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 patchlevel.h
--- gnu/usr.bin/perl/patchlevel.h   5 Sep 2014 06:53:07 -   1.34
+++ gnu/usr.bin/perl/patchlevel.h   24 Oct 2014 20:25:05 -
@@ -134,6 +134,7 @@ hunk.
 static const char * const local_patches[] = {
NULL
,Update libnet to 1.27
+   ,CVE-2014-4330
 #ifdef PERL_GIT_UNCOMMITTED_CHANGES
,uncommitted-changes
 #endif
Index: gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm
===
RCS file: 
/data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm,v
retrieving revision 1.1.1.3
diff -u -p -u -p -r1.1.1.3 Dumper.pm
--- gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm 24 Mar 2014 14:58:59 -  
1.1.1.3
+++ gnu/usr.bin/perl/dist/Data-Dumper/Dumper.pm 24 Oct 2014 20:19:35 -
@@ -56,6 +56,7 @@ $Useperl= 0 unless defined $
 $Sortkeys   = 0 unless defined $Sortkeys;
 $Deparse= 0 unless defined $Deparse;
 $Sparseseen = 0 unless defined $Sparseseen;
+$Maxrecurse = 1000  unless defined $Maxrecurse;
 
 #
 # expects an arrayref of values to be dumped.
@@ -92,6 +93,7 @@ sub new {
 'bless'= $Bless,# keyword to use for bless
 #expdepth   = $Expdepth,   # cutoff depth for explicit dumping
 maxdepth   = $Maxdepth,   # depth beyond which we give up
+   maxrecurse = $Maxrecurse, # depth beyond which we abort
 useperl= $Useperl,# use the pure Perl implementation
 sortkeys   = $Sortkeys,   # flag or filter for sorting hash keys
 deparse= $Deparse,# use B::Deparse for coderefs
@@ -351,6 +353,12 @@ sub _dump {
   return qq['$val'];
 }
 
+# avoid recursing infinitely [perl #122111]
+if ($s-{maxrecurse}  0
+and $s-{level} = $s-{maxrecurse}) {
+die Recursion limit of $s-{maxrecurse} exceeded;
+}
+
 # we have a blessed ref
 my ($blesspad);
 if ($realpack and !$no_bless) {
@@ -683,6 +691,11 @@ sub Maxdepth {
   defined($v) ? (($s-{'maxdepth'} = $v), return $s) : $s-{'maxdepth'};
 }
 
+sub Maxrecurse {
+  my($s, $v) = @_;
+  defined($v) ? (($s-{'maxrecurse'} = $v), return $s) : $s-{'maxrecurse'};
+}
+
 sub Useperl {
   my($s, $v) = @_;
   defined($v) ? (($s-{'useperl'} = $v), return $s) : $s-{'useperl'};
@@ -1105,6 +1118,16 @@ we don't venture into a structure.  Has 
 CData::Dumper::Purity is set.  (Useful in debugger when we often don't
 want to see more than enough).  Default is 0, which means there is
 no maximum depth.
+
+=item *
+
+$Data::Dumper::Maxrecurse  Ior  $IOBJ-Maxrecurse(I[NEWVAL])
+
+Can be set to a positive integer that specifies the depth beyond which
+recursion into a structure will throw an exception.  This is intended
+as a security measure to prevent perl running out of stack space when
+dumping an excessively deep structure.  Can be set to 0 to remove the
+limit.  Default is 1000.
 
 =item *
 
Index: gnu/usr.bin/perl/dist/Data-Dumper/Dumper.xs
===
RCS file: 
/data/mirror/openbsd/cvs/src/gnu/usr.bin/perl/dist/Data-Dumper/Dumper.xs,v
retrieving revision 1.1.1.3
diff -u -p -u -p -r1.1.1.3 Dumper.xs
--- gnu/usr.bin/perl/dist/Data-Dumper/Dumper.xs 24 Mar 2014 14:58:59 -  
1.1.1.3
+++ gnu/usr.bin/perl/dist/Data-Dumper/Dumper.xs 24 Oct 2014 20:22:57 -
@@ -26,7 +26,8 @@ static I32 DD_dump (pTHX_ SV *val, const
SV *pad, SV *xpad, SV *apad, SV *sep, SV *pair,
SV *freezer, SV