This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Perl_sv_vcatpvfn_flags: simplify wrap checking
authorDavid Mitchell <davem@iabyn.com>
Tue, 9 May 2017 14:55:07 +0000 (15:55 +0100)
committerDavid Mitchell <davem@iabyn.com>
Wed, 7 Jun 2017 08:11:00 +0000 (09:11 +0100)
The main SvGROW() has a new-length arg roughly equivalent to

    (SvCUR(sv) + elen + zeros + esignlen + dotstrlen + 1);

Rationalise the overflow/wrap checking by doing each individual addition
separately with its own check. This is slightly redundant as some of the
values are interdependent, but this way it's easier to see whether all
possible overflows are being checked for.

`

sv.c

diff --git a/sv.c b/sv.c
index 11bd021..285b7a6 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -13040,23 +13040,43 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
        }
 
 
        }
 
 
-        /* append esignbuf, filler, zeroes, eptr and dotstr to sv */
+        /* append esignbuf, filler, zeros, eptr and dotstr to sv */
 
         {
             STRLEN need, have, gap;
 
             /* signed value that's wrapped? */
             assert(elen  <= ((~(STRLEN)0) >> 1));
 
         {
             STRLEN need, have, gap;
 
             /* signed value that's wrapped? */
             assert(elen  <= ((~(STRLEN)0) >> 1));
-            have = esignlen + zeros + elen;
-            if (have < zeros)
+
+            /* Most of these length vars can range to any value if
+             * supplied with a hostile format and/or args. So check every
+             * addition for possible overflow. In reality some of these
+             * values are interdependent so these checks are slightly
+             * redundant. But its easier to be certain this way.
+             */
+
+            have = elen;
+
+            if (have >= (((STRLEN)~0) - zeros))
+                croak_memory_wrap();
+            have += zeros;
+
+            if (have >= (((STRLEN)~0) - esignlen))
                 croak_memory_wrap();
                 croak_memory_wrap();
+            have += esignlen;
 
             need = (have > width ? have : width);
             gap = need - have;
 
 
             need = (have > width ? have : width);
             gap = need - have;
 
-            if (need >= (((STRLEN)~0) - SvCUR(sv) - dotstrlen - 1))
+            if (need >= (((STRLEN)~0) - dotstrlen))
+                croak_memory_wrap();
+            need += dotstrlen;
+
+            if (need >= (((STRLEN)~0) - (SvCUR(sv) + 1)))
                 croak_memory_wrap();
                 croak_memory_wrap();
-            SvGROW(sv, SvCUR(sv) + need + dotstrlen + 1);
+            need += (SvCUR(sv) + 1);
+
+            SvGROW(sv, need);
 
             p = SvEND(sv);
             if (esignlen && fill == '0') {
 
             p = SvEND(sv);
             if (esignlen && fill == '0') {