This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
avoid integer overflow in Perl_av_extend_guts()
authorDavid Mitchell <davem@iabyn.com>
Sun, 21 Dec 2014 00:40:13 +0000 (00:40 +0000)
committerDavid Mitchell <davem@iabyn.com>
Wed, 31 Dec 2014 11:28:51 +0000 (11:28 +0000)
There were two issues; first the 'overextend' algorithm (add a fifth of
the current size to the requested size) could overflow,
and secondly MEM_WRAP_CHECK_1() was being called with newmax+1,
which could overflow if newmax happened to equal SSize_t_MAX.

e.g.

    $a[0x7fffffffffffffff] = 1
    $a[5] = 1; $a[0x7fffffffffffffff] = 1

could produce under ASan:

    av.c:133:16: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long'
    av.c:170:7: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long'

av.c

diff --git a/av.c b/av.c
index 3de7b83..53690d5 100644 (file)
--- a/av.c
+++ b/av.c
@@ -130,14 +130,23 @@ Perl_av_extend_guts(pTHX_ AV *av, SSize_t key, SSize_t *maxp, SV ***allocp,
                if (key <= newmax) 
                    goto resized;
 #endif 
-               newmax = key + *maxp / 5;
+                /* overflow-safe version of newmax = key + *maxp/5 */
+               newmax = *maxp / 5;
+                newmax = (key > SSize_t_MAX - newmax)
+                            ? SSize_t_MAX : key + newmax;
              resize:
                {
 #ifdef PERL_MALLOC_WRAP /* Duplicated in pp_hot.c */
                    static const char oom_array_extend[] =
                        "Out of memory during array extend";
 #endif
-                   MEM_WRAP_CHECK_1(newmax+1, SV*, oom_array_extend);
+                    /* it should really be newmax+1 here, but if newmax
+                     * happens to equal SSize_t_MAX, then newmax+1 is
+                     * undefined. This means technically we croak one
+                     * index lower than we should in theory; in practice
+                     * its unlikely the system has SSize_t_MAX/sizeof(SV*)
+                     * bytes to spare! */
+                   MEM_WRAP_CHECK_1(newmax, SV*, oom_array_extend);
                }
 #ifdef STRESS_REALLOC
                {
@@ -167,7 +176,8 @@ Perl_av_extend_guts(pTHX_ AV *av, SSize_t key, SSize_t *maxp, SV ***allocp,
                    static const char oom_array_extend[] =
                        "Out of memory during array extend";
 #endif
-                   MEM_WRAP_CHECK_1(newmax+1, SV*, oom_array_extend);
+                    /* see comment above about newmax+1*/
+                   MEM_WRAP_CHECK_1(newmax, SV*, oom_array_extend);
                }
                Newx(*allocp, newmax+1, SV*);
                ary = *allocp + 1;