From d9c4b4ae5a1a17347ff5e3ecbf8e1d9da481f476 Mon Sep 17 00:00:00 2001
From: David Mitchell <davem@iabyn.com>
Date: Wed, 3 Apr 2019 13:23:24 +0100
Subject: [PATCH] Data::Dumper - avoid leak on croak
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

v5.21.3-742-g19be3be696 added a facility to Dumper.xs to croak if the
recursion level became too deep (1000 by default).

The trouble with this is that various parts of DD_dump() allocate
temporary SVs and buffers, which will leak if DD_dump() unceremoniously
just croaks().

This currently manifests as dist/Data-Dumper/t/recurse.t failing under
Address Sanitiser.

This commit makes the depth checking code just set a sticky 'too deep'
boolean flag, and
a) on entry, DD_dump() just returns immediately if the flag is set;
b) the flag is checked by the top-level called of DD_dump() and croaks
if set.

So the net effect is to defer croaking until the dump is complete,
and avoid any further recursion once the flag is set.

This is a bit of a quick fix. More long-term solutions would be to
convert DD_dump() to be iterative rather than recursive, and/or make
sure all temporary SVs and buffers are suitably anchored somewhere so
that they get cleaned up on croak.

Petr Písař: Ported from 6d65cb5d847ac93680949c4fa02111808207fbdc in
perl git tree.

Signed-off-by: Petr Písař <ppisar@redhat.com>
---
 Dumper.pm |  6 +++---
 Dumper.xs | 27 ++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/Dumper.pm b/Dumper.pm
index 40aeb7d..06af4c4 100644
--- a/Dumper.pm
+++ b/Dumper.pm
@@ -10,7 +10,7 @@
 package Data::Dumper;
 
 BEGIN {
-    $VERSION = '2.173'; # Don't forget to set version and release
+    $VERSION = '2.174'; # Don't forget to set version and release
 }               # date in POD below!
 
 #$| = 1;
@@ -1461,13 +1461,13 @@ be to use the C<Sortkeys> filter of Data::Dumper.
 
 Gurusamy Sarathy        gsar@activestate.com
 
-Copyright (c) 1996-2017 Gurusamy Sarathy. All rights reserved.
+Copyright (c) 1996-2019 Gurusamy Sarathy. All rights reserved.
 This program is free software; you can redistribute it and/or
 modify it under the same terms as Perl itself.
 
 =head1 VERSION
 
-Version 2.173
+Version 2.174
 
 =head1 SEE ALSO
 
diff --git a/Dumper.xs b/Dumper.xs
index 7f0b027..a324cb6 100644
--- a/Dumper.xs
+++ b/Dumper.xs
@@ -61,9 +61,10 @@
 #endif
 
 /* This struct contains almost all the user's desired configuration, and it
- * is treated as constant by the recursive function. This arrangement has
- * the advantage of needing less memory than passing all of them on the
- * stack all the time (as was the case in an earlier implementation). */
+ * is treated as mostly constant (except for maxrecursed) by the recursive
+ * function.  This arrangement has the advantage of needing less memory
+ * than passing all of them on the stack all the time (as was the case in
+ * an earlier implementation). */
 typedef struct {
     SV *pad;
     SV *xpad;
@@ -74,6 +75,7 @@ typedef struct {
     SV *toaster;
     SV *bless;
     IV maxrecurse;
+    bool maxrecursed; /* at some point we exceeded the maximum recursion level */
     I32 indent;
     I32 purity;
     I32 deepcopy;
@@ -97,7 +99,7 @@ static bool safe_decimal_number(const char *p, STRLEN len);
 static SV *sv_x (pTHX_ SV *sv, const char *str, STRLEN len, I32 n);
 static I32 DD_dump (pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval,
                     HV *seenhv, AV *postav, const I32 level, SV *apad,
-                    const Style *style);
+                    Style *style);
 
 #ifndef HvNAME_get
 #define HvNAME_get HvNAME
@@ -615,7 +617,7 @@ deparsed_output(pTHX_ SV *val)
  */
 static I32
 DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
-	AV *postav, const I32 level, SV *apad, const Style *style)
+	AV *postav, const I32 level, SV *apad, Style *style)
 {
     char tmpbuf[128];
     Size_t i;
@@ -642,6 +644,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
     if (!val)
 	return 0;
 
+    if (style->maxrecursed)
+        return 0;
+
     /* If the output buffer has less than some arbitrary amount of space
        remaining, then enlarge it. For the test case (25M of output),
        *1.1 was slower, *2.0 was the same, so the first guess of 1.5 is
@@ -793,7 +798,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	}
 
         if (style->maxrecurse > 0 && level >= style->maxrecurse) {
-            croak("Recursion limit of %" IVdf " exceeded", style->maxrecurse);
+            style->maxrecursed = TRUE;
 	}
 
 	if (realpack && !no_bless) {				/* we have a blessed ref */
@@ -1528,6 +1533,7 @@ Data_Dumper_Dumpxs(href, ...)
             style.indent = 2;
             style.quotekeys = 1;
             style.maxrecurse = 1000;
+            style.maxrecursed = FALSE;
             style.purity = style.deepcopy = style.useqq = style.maxdepth
                 = style.use_sparse_seen_hash = style.trailingcomma = 0;
             style.pad = style.xpad = style.sep = style.pair = style.sortkeys
@@ -1675,7 +1681,7 @@ Data_Dumper_Dumpxs(href, ...)
 		    DD_dump(aTHX_ val, SvPVX_const(name), SvCUR(name), valstr, seenhv,
                             postav, 0, newapad, &style);
 		    SPAGAIN;
-		
+
                     if (style.indent >= 2 && !terse)
 			SvREFCNT_dec(newapad);
 
@@ -1715,6 +1721,13 @@ Data_Dumper_Dumpxs(href, ...)
 		}
 		SvREFCNT_dec(postav);
 		SvREFCNT_dec(valstr);
+
+                /* we defer croaking until here so that temporary SVs and
+                 * buffers won't be leaked */
+                if (style.maxrecursed)
+                    croak("Recursion limit of %" IVdf " exceeded",
+                            style.maxrecurse);
+		
 	    }
 	    else
 		croak("Call to new() method failed to return HASH ref");
-- 
2.20.1