- creating an image where the size of the allocated image buffer would

  overflow an integer would cause too small a buffer to be allocated.
  This could potentially be a security hole.
  partly resolves https://rt.cpan.org/Ticket/Display.html?id=8213

diff --git a/Changes b/Changes
index 7eb76464faabe5453960316aee14a696e6d5b4d5..5a356ac82138d3b30ed478b5619027081700f250 100644 (file)
--- a/Changes
+++ b/Changes
@@ -884,6 +884,10 @@ Revision history for Perl extension Imager.
 - updated download locations for the various libraries that Imager 
   depends on.  Added some advice for cygwin.
 - more information on gif library versions in README and Makefile.PL
+- creating an image where the size of the allocated image buffer would
+  overflow an integer would cause too small a buffer to be allocated.
+  This could potentially be a security hole.
+  partly resolves https://rt.cpan.org/Ticket/Display.html?id=8213
 
 =================================================================
 

diff --git a/image.c b/image.c
index 8dfdb092fa11437dec279199bb2773a0564d75e0..eba037c5d0c1a1bcb55e8bff032b1cc6da519a7d 100644 (file)
--- a/image.c
+++ b/image.c
@@ -348,6 +348,8 @@ Re-new image reference
 
 i_img *
 i_img_empty_ch(i_img *im,int x,int y,int ch) {
+  int bytes;
+
   mm_log((1,"i_img_empty_ch(*im %p, x %d, y %d, ch %d)\n", im, x, y, ch));
 
   if (x < 1 || y < 1) {
@@ -358,6 +360,12 @@ i_img_empty_ch(i_img *im,int x,int y,int ch) {
     i_push_errorf(0, "channels must be between 1 and %d", MAXCHANNELS);
     return NULL;
   }
+  /* check this multiplication doesn't overflow */
+  bytes = x*y*ch;
+  if (bytes / y / ch != x) {
+    i_push_errorf(0, "integer overflow calculating image allocation");
+    return NULL;
+  }
 
   if (im == NULL)
     if ( (im=mymalloc(sizeof(i_img))) == NULL)
@@ -369,8 +377,9 @@ i_img_empty_ch(i_img *im,int x,int y,int ch) {
   im->ysize    = y;
   im->channels = ch;
   im->ch_mask  = MAXINT;
-  im->bytes=x*y*im->channels;
-  if ( (im->idata=mymalloc(im->bytes)) == NULL) m_fatal(2,"malloc() error\n"); 
+  im->bytes=bytes;
+  if ( (im->idata=mymalloc(im->bytes)) == NULL) 
+    m_fatal(2,"malloc() error\n"); 
   memset(im->idata,0,(size_t)im->bytes);
   
   im->ext_data = NULL;

diff --git a/img16.c b/img16.c
index bb5847561e97056e4552d905a2968d5d2fe17680..2d795477fbf172662c0c56112cc7917c9e4916c7 100644 (file)
--- a/img16.c
+++ b/img16.c
@@ -143,6 +143,7 @@ Creates a new 16-bit per sample image.
 =cut
 */
 i_img *i_img_16_new_low(i_img *im, int x, int y, int ch) {
+  int bytes;
   mm_log((1,"i_img_16_new(x %d, y %d, ch %d)\n", x, y, ch));
 
   if (x < 1 || y < 1) {
@@ -153,13 +154,18 @@ i_img *i_img_16_new_low(i_img *im, int x, int y, int ch) {
     i_push_errorf(0, "channels must be between 1 and %d", MAXCHANNELS);
     return NULL;
   }
+  bytes =  x * y * ch * 2;
+  if (bytes / y / ch / 2 != x) {
+    i_push_errorf(0, "integer overflow calculating image allocation");
+    return NULL;
+  }
   
   *im = IIM_base_16bit_direct;
   i_tags_new(&im->tags);
   im->xsize = x;
   im->ysize = y;
   im->channels = ch;
-  im->bytes = x * y * ch * 2;
+  im->bytes = bytes;
   im->ext_data = NULL;
   im->idata = mymalloc(im->bytes);
   if (im->idata) {

diff --git a/imgdouble.c b/imgdouble.c
index d8e852f02a991363925a8db4d91bda6e48c50f23..8dc4ccc6f85e1ac52b702b8f66071bf5afb58f9b 100644 (file)
--- a/imgdouble.c
+++ b/imgdouble.c
@@ -86,6 +86,8 @@ Creates a new double per sample image.
 =cut
 */
 i_img *i_img_double_new_low(i_img *im, int x, int y, int ch) {
+  int bytes;
+
   mm_log((1,"i_img_double_new(x %d, y %d, ch %d)\n", x, y, ch));
 
   if (x < 1 || y < 1) {
@@ -96,13 +98,18 @@ i_img *i_img_double_new_low(i_img *im, int x, int y, int ch) {
     i_push_errorf(0, "channels must be between 1 and %d", MAXCHANNELS);
     return NULL;
   }
+  bytes = x * y * ch * sizeof(double);
+  if (bytes / y / ch / sizeof(double) != x) {
+    i_push_errorf(0, "integer overflow calculating image allocation");
+    return NULL;
+  }
   
   *im = IIM_base_double_direct;
   i_tags_new(&im->tags);
   im->xsize = x;
   im->ysize = y;
   im->channels = ch;
-  im->bytes = x * y * ch * sizeof(double);
+  im->bytes = bytes;
   im->ext_data = NULL;
   im->idata = mymalloc(im->bytes);
   if (im->idata) {

diff --git a/palimg.c b/palimg.c
index f397807c5baf7f7e117306b985f6a6d6d995cb55..ecb14660c1a4c5e07054225d19fd2f1729b8a5d5 100644 (file)
--- a/palimg.c
+++ b/palimg.c
@@ -84,6 +84,7 @@ Currently 0 < maxpal <= 256
 */
 i_img *i_img_pal_new_low(i_img *im, int x, int y, int channels, int maxpal) {
   i_img_pal_ext *palext;
+  int bytes;
 
   i_clear_error();
   if (maxpal < 0 || maxpal > 256) {
@@ -98,6 +99,11 @@ i_img *i_img_pal_new_low(i_img *im, int x, int y, int channels, int maxpal) {
     i_push_errorf(0, "Channels must be positive and <= %d", MAXCHANNELS);
     return NULL;
   }
+  bytes = sizeof(i_palidx) * x * y;
+  if (bytes / y / sizeof(i_palidx) != x) {
+    i_push_errorf(0, "integer overflow calculating image allocation");
+    return NULL;
+  }
 
   memcpy(im, &IIM_base_8bit_pal, sizeof(i_img));
   palext = mymalloc(sizeof(i_img_pal_ext));
@@ -107,7 +113,7 @@ i_img *i_img_pal_new_low(i_img *im, int x, int y, int channels, int maxpal) {
   palext->last_found = -1;
   im->ext_data = palext;
   i_tags_new(&im->tags);
-  im->bytes = sizeof(i_palidx) * x * y;
+  im->bytes = bytes;
   im->idata = mymalloc(im->bytes);
   im->channels = channels;
   memset(im->idata, 0, im->bytes);

diff --git a/t/t01introvert.t b/t/t01introvert.t
index 8e9c29f1de83f66bc1044f67516429dc5ee9b16a..b65ea8598fcecaf4debc496c711ae347377d8210 100644 (file)
--- a/t/t01introvert.t
+++ b/t/t01introvert.t
@@ -8,7 +8,7 @@
 use strict;
 
 my $loaded;
-BEGIN { $| = 1; print "1..85\n"; }
+BEGIN { $| = 1; print "1..93\n"; }
 END {print "not ok 1\n" unless $loaded;}
 use Imager qw(:handy :all);
 $loaded = 1;
@@ -250,6 +250,49 @@ okn($num++, !Imager->new(xsize=>1, ysize=>1, channels=>5),
 matchn($num++, Imager->errstr, qr/channels must be between 1 and 4/,
        "out of range channel message check");
 
+{
+  # https://rt.cpan.org/Ticket/Display.html?id=8213
+  # check for handling of memory allocation of very large images
+  # only test this on 32-bit machines - on a 64-bit machine it may
+  # result in trying to allocate 4Gb of memory, which is unfriendly at
+  # least and may result in running out of memory, causing a different
+  # type of exit
+  use Config;
+  if ($Config{ivsize} == 4) {
+    my $uint_range = 256 ** $Config{ivsize};
+    print "# range $uint_range\n";
+    my $dim1 = int(sqrt($uint_range))+1;
+    
+    my $im_b = Imager->new(xsize=>$dim1, ysize=>$dim1, channels=>1);
+    isn($num++, $im_b, undef, "integer overflow check - 1 channel");
+    
+    $im_b = Imager->new(xisze=>$dim1, ysize=>1, channels=>1);
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim1, channels=>1);
+    okn($num++, $im_b, "but same height ok");
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+
+    # do a similar test with a 3 channel image, so we're sure we catch
+    # the same case where the third dimension causes the overflow
+    my $dim3 = int(sqrt($uint_range / 3))+1;
+    
+    $im_b = Imager->new(xsize=>$dim3, ysize=>$dim3, channels=>3);
+    isn($num++, $im_b, undef, "integer overflow check - 3 channel");
+    
+    $im_b = Imager->new(xisze=>$dim3, ysize=>1, channels=>3);
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim3, channels=>3);
+    okn($num++, $im_b, "but same height ok");
+
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+  }
+  else {
+    skipn($num, 8, "don't want to allocate 4Gb");
+    $num += 8;
+  }
+}
 
 sub check_add {
   my ($base, $im, $color, $expected) = @_;

diff --git a/t/t021sixteen.t b/t/t021sixteen.t
index d1a7c46c0dfc15e62293a625f91e790c54c24cd7..9ae5bcf3e6a83b8763adec6c4aab959568510ac2 100644 (file)
--- a/t/t021sixteen.t
+++ b/t/t021sixteen.t
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
-BEGIN { $| = 1; print "1..43\n"; }
+BEGIN { $| = 1; print "1..51\n"; }
 my $loaded;
 END {print "not ok 1\n" unless $loaded;}
 use Imager qw(:all :handy);
@@ -108,6 +108,50 @@ okn($num++, !Imager->new(xsize=>1, ysize=>1, bits=>16, channels=>5),
 matchn($num++, Imager->errstr, qr/channels must be between 1 and 4/,
        "and correct error message");
 
+{
+  # https://rt.cpan.org/Ticket/Display.html?id=8213
+  # check for handling of memory allocation of very large images
+  # only test this on 32-bit machines - on a 64-bit machine it may
+  # result in trying to allocate 4Gb of memory, which is unfriendly at
+  # least and may result in running out of memory, causing a different
+  # type of exit
+  use Config;
+  if ($Config{ivsize} == 4) {
+    my $uint_range = 256 ** $Config{ivsize};
+    print "# range $uint_range\n";
+    my $dim1 = int(sqrt($uint_range/2))+1;
+    
+    my $im_b = Imager->new(xsize=>$dim1, ysize=>$dim1, channels=>1, bits=>16);
+    isn($num++, $im_b, undef, "integer overflow check - 1 channel");
+    
+    $im_b = Imager->new(xisze=>$dim1, ysize=>1, channels=>1, bits=>16);
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim1, channels=>1, bits=>16);
+    okn($num++, $im_b, "but same height ok");
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+
+    # do a similar test with a 3 channel image, so we're sure we catch
+    # the same case where the third dimension causes the overflow
+    my $dim3 = int(sqrt($uint_range / 3 / 2))+1;
+    
+    $im_b = Imager->new(xsize=>$dim3, ysize=>$dim3, channels=>3, bits=>16);
+    isn($num++, $im_b, undef, "integer overflow check - 3 channel");
+    
+    $im_b = Imager->new(xisze=>$dim3, ysize=>1, channels=>3, bits=>16);
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim3, channels=>3, bits=>16);
+    okn($num++, $im_b, "but same height ok");
+
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+  }
+  else {
+    skipn($num, 8, "don't want to allocate 4Gb");
+    $num += 8;
+  }
+}
+
 sub NCF {
   return Imager::Color::Float->new(@_);
 }

diff --git a/t/t022double.t b/t/t022double.t
index 87fdbbb6994e894d288d65d3d7e375282f7b5ea9..b3d86960f8bd3ce647fc5f561bb49da4294daea9 100644 (file)
--- a/t/t022double.t
+++ b/t/t022double.t
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
-BEGIN { $| = 1; print "1..42\n"; }
+BEGIN { $| = 1; print "1..50\n"; }
 my $loaded;
 END {print "not ok 1\n" unless $loaded;}
 use Imager qw(:all :handy);
@@ -98,6 +98,50 @@ okn($num++, !Imager->new(xsize=>1, ysize=>1, bits=>'double', channels=>5),
 matchn($num++, Imager->errstr, qr/channels must be between 1 and 4/,
        "and correct message");
 
+{
+  # https://rt.cpan.org/Ticket/Display.html?id=8213
+  # check for handling of memory allocation of very large images
+  # only test this on 32-bit machines - on a 64-bit machine it may
+  # result in trying to allocate 4Gb of memory, which is unfriendly at
+  # least and may result in running out of memory, causing a different
+  # type of exit
+  use Config;
+  if ($Config{ivsize} == 4) {
+    my $uint_range = 256 ** $Config{ivsize};
+    my $dbl_size = $Config{doublesize} || 8;
+    my $dim1 = int(sqrt($uint_range/$dbl_size))+1;
+    
+    my $im_b = Imager->new(xsize=>$dim1, ysize=>$dim1, channels=>1, bits=>'double');
+    isn($num++, $im_b, undef, "integer overflow check - 1 channel");
+    
+    $im_b = Imager->new(xisze=>$dim1, ysize=>1, channels=>1, bits=>'double');
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim1, channels=>1, bits=>'double');
+    okn($num++, $im_b, "but same height ok");
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+
+    # do a similar test with a 3 channel image, so we're sure we catch
+    # the same case where the third dimension causes the overflow
+    my $dim3 = int(sqrt($uint_range / 3 / $dbl_size))+1;
+    
+    $im_b = Imager->new(xsize=>$dim3, ysize=>$dim3, channels=>3, bits=>'double');
+    isn($num++, $im_b, undef, "integer overflow check - 3 channel");
+    
+    $im_b = Imager->new(xisze=>$dim3, ysize=>1, channels=>3, bits=>'double');
+    okn($num++, $im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim3, channels=>3, bits=>'double');
+    okn($num++, $im_b, "but same height ok");
+
+    matchn($num++, Imager->errstr, qr/integer overflow/,
+           "check the error message");
+  }
+  else {
+    skipn($num, 8, "don't want to allocate 4Gb");
+    $num += 8;
+  }
+}
+
 sub NCF {
   return Imager::Color::Float->new(@_);
 }

diff --git a/t/t023palette.t b/t/t023palette.t
index a247081444b6a45e592f41714863fc4488a8678f..57e25cf2a7c2c3fcb6d9a1a250e5015eb7c67a9b 100644 (file)
--- a/t/t023palette.t
+++ b/t/t023palette.t
@@ -4,7 +4,7 @@ use strict;
 my $loaded;
 BEGIN { 
   require "t/testtools.pl";
-  $| = 1; print "1..49\n";
+  $| = 1; print "1..57\n";
 }
 END { okx(0, "loading") unless $loaded; }
 use Imager;
@@ -136,6 +136,50 @@ okx(!Imager->new(xsize=>1, ysize=>1, type=>'paletted', channels=>5),
 matchx(Imager->errstr, qr/Channels must be positive and <= 4/,
        "and correct error message");
 
+{
+  # https://rt.cpan.org/Ticket/Display.html?id=8213
+  # check for handling of memory allocation of very large images
+  # only test this on 32-bit machines - on a 64-bit machine it may
+  # result in trying to allocate 4Gb of memory, which is unfriendly at
+  # least and may result in running out of memory, causing a different
+  # type of exit
+  use Config;
+  if ($Config{ivsize} == 4) {
+    my $uint_range = 256 ** $Config{ivsize};
+    my $dim1 = int(sqrt($uint_range))+1;
+    
+    my $im_b = Imager->new(xsize=>$dim1, ysize=>$dim1, channels=>1, type=>'paletted');
+    isx($im_b, undef, "integer overflow check - 1 channel");
+    
+    $im_b = Imager->new(xisze=>$dim1, ysize=>1, channels=>1, type=>'paletted');
+    okx($im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim1, channels=>1, type=>'paletted');
+    okx($im_b, "but same height ok");
+    matchx(Imager->errstr, qr/integer overflow/,
+           "check the error message");
+
+    # do a similar test with a 3 channel image, so we're sure we catch
+    # the same case where the third dimension causes the overflow
+    # for paletted images the third dimension can't cause an overflow
+    # but make sure we didn't anything too dumb in the checks
+    my $dim3 = $dim1;
+    
+    $im_b = Imager->new(xsize=>$dim3, ysize=>$dim3, channels=>3, type=>'paletted');
+    isx($im_b, undef, "integer overflow check - 3 channel");
+    
+    $im_b = Imager->new(xisze=>$dim3, ysize=>1, channels=>3, type=>'paletted');
+    okx($im_b, "but same width ok");
+    $im_b = Imager->new(xisze=>1, ysize=>$dim3, channels=>3, type=>'paletted');
+    okx($im_b, "but same height ok");
+
+    matchx(Imager->errstr, qr/integer overflow/,
+           "check the error message");
+  }
+  else {
+    skipx(8, "don't want to allocate 4Gb");
+  }
+}
+
 sub coloreq {
   my ($left, $right, $comment) = @_;