- fixed a related problem for image fills.
authorTony Cook <tony@develop=help.com>
Wed, 23 Apr 2008 01:00:02 +0000 (01:00 +0000)
committerTony Cook <tony@develop=help.com>
Wed, 23 Apr 2008 01:00:02 +0000 (01:00 +0000)
 - Possible security issue: The floating point sample path for image
   based fills had a buffer overflow.  This would overwrite the end of
   a malloc()ed buffer with double precision floats.

Changes
fills.c
t/t20fill.t

diff --git a/Changes b/Changes
index e9eb06f..2e60632 100644 (file)
--- a/Changes
+++ b/Changes
@@ -22,6 +22,12 @@ Bug fixes:
    color channels from the supplied fg/bg colors.
    https://rt.cpan.org/Ticket/Display.html?id=35278
 
+ - fixed a related problem for image fills.
+
+ - Possible security issue: The floating point sample path for image
+   based fills had a buffer overflow.  This would overwrite the end of
+   a malloc()ed buffer with double precision floats.
+
 Imager 0.63 - 7 April 2008
 ===========
 
diff --git a/fills.c b/fills.c
index 973b2f9..ac9660a 100644 (file)
--- a/fills.c
+++ b/fills.c
@@ -756,6 +756,7 @@ static void fill_image(i_fill_t *fill, int x, int y, int width, int channels,
   struct i_fill_image_t *f = (struct i_fill_image_t *)fill;
   int i = 0;
   i_color *out = data;
+  int want_channels = channels > 2 ? 4 : 2;
   
   if (f->has_matrix) {
     /* the hard way */
@@ -818,33 +819,12 @@ static void fill_image(i_fill_t *fill, int x, int y, int width, int channels,
       ++i;
     }
   }
-  if (f->src->channels == 3) {
-    /* just set the alpha */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = 255;
-      data++;
-    }
-  }
-  else if (f->src->channels == 2) {
-    /* copy the alpha to channel 3, duplicate the grey value */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = data->channel[1];
-      data->channel[1] = data->channel[2] = data->channel[0];
-      data++;
-    }
-  }
-  else if (f->src->channels == 1) {
-    /* set the alpha, duplicate grey */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = 255;
-      data->channel[1] = data->channel[2] = data->channel[0];
-      data++;
-    }
-  }
+  if (f->src->channels != want_channels)
+    i_adapt_colors(want_channels, f->src->channels, data, width);
 }
 
 /*
-=item fill_image(fill, x, y, width, channels, data, work)
+=item fill_imagef(fill, x, y, width, channels, data, work)
 
 =cut
 */
@@ -852,8 +832,10 @@ static void fill_imagef(i_fill_t *fill, int x, int y, int width, int channels,
                        i_fcolor *data) {
   struct i_fill_image_t *f = (struct i_fill_image_t *)fill;
   int i = 0;
+  int want_channels = channels > 2 ? 4 : 2;
   
   if (f->has_matrix) {
+    i_fcolor *work_data = data;
     /* the hard way */
     while (i < width) {
       double rx = f->matrix[0] * (x+i) + f->matrix[1] * y + f->matrix[2];
@@ -886,11 +868,12 @@ static void fill_imagef(i_fill_t *fill, int x, int y, int width, int channels,
         }
         c2[dy] = interp_i_fcolor(c[dy][0], c[dy][1], rx, f->src->channels);
       }
-      *data++ = interp_i_fcolor(c2[0], c2[1], ry, f->src->channels);
+      *work_data++ = interp_i_fcolor(c2[0], c2[1], ry, f->src->channels);
       ++i;
     }
   }
   else {
+    i_fcolor *work_data = data;
     /* the easy way */
     /* this should be possible to optimize to use i_glin() */
     while (i < width) {
@@ -909,34 +892,13 @@ static void fill_imagef(i_fill_t *fill, int x, int y, int width, int channels,
       }
       rx -= ix * f->src->xsize;
       ry -= iy * f->src->ysize;
-      i_gpixf(f->src, rx, ry, data);
-      ++data;
+      i_gpixf(f->src, rx, ry, work_data);
+      ++work_data;
       ++i;
     }
   }
-  if (f->src->channels == 3) {
-    /* just set the alpha */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = 1.0;
-      data++;
-    }
-  }
-  else if (f->src->channels == 2) {
-    /* copy the alpha to channel 3, duplicate the grey value */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = data->channel[1];
-      data->channel[1] = data->channel[2] = data->channel[0];
-      data++;
-    }
-  }
-  else if (f->src->channels == 1) {
-    /* set the alpha, duplicate grey */
-    for (i = 0; i <  width; ++i) {
-      data->channel[3] = 1.0;
-      data->channel[1] = data->channel[2] = data->channel[0];
-      data++;
-    }
-  }
+  if (f->src->channels != want_channels)
+    i_adapt_fcolors(want_channels, f->src->channels, data, width);
 }
 
 
index f42dc49..ff6471f 100644 (file)
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
-use Test::More tests => 123;
+use Test::More tests => 129;
 
 use Imager ':handy';
 use Imager::Fill;
@@ -431,7 +431,38 @@ SKIP:
     $im_c->box(filled => 1, color => 'FFFFFF');
     $im_c->box(fill => $fill);
     my $im_cg = $im_g->convert(preset => 'rgb');
-    is_image($im_c, $im_cg, "check hatch is the same between color and greyscale");
+    is_image($im_c, $im_cg, "check hatch is the same between color and greyscale (bits $bits)");
+
+    # check the same for image fills
+    my $grey_fill = Imager::Fill->new
+      (
+       image => $im_g, 
+       combine => 'normal'
+      );
+    my $im_cfg = Imager->new(xsize => 20, ysize => 20, bits => $bits);
+    $im_cfg->box(filled => 1, color => '808080');
+    $im_cfg->box(fill => $grey_fill);
+    my $rgb_fill = Imager::Fill->new
+      (
+       image => $im_cg, 
+       combine => 'normal'
+      );
+    my $im_cfc = Imager->new(xsize => 20, ysize => 20, bits => $bits);
+    $im_cfc->box(filled => 1, color => '808080');
+    $im_cfc->box(fill => $rgb_fill);
+    is_image($im_cfg, $im_cfc, "check filling from grey image matches filling from rgb (bits = $bits)");
+
+    my $im_gfg = Imager->new(xsize => 20, ysize => 20, channels => 1, bits => $bits);
+    $im_gfg->box(filled => 1, color => '808080');
+    $im_gfg->box(fill => $grey_fill);
+    my $im_gfg_c = $im_gfg->convert(preset => 'rgb');
+    is_image($im_gfg_c, $im_cfg, "check grey filled with grey against base (bits = $bits)");
+
+    my $im_gfc = Imager->new(xsize => 20, ysize => 20, channels => 1, bits => $bits);
+    $im_gfc->box(filled => 1, color => '808080');
+    $im_gfc->box(fill => $rgb_fill);
+    my $im_gfc_c = $im_gfc->convert(preset => 'rgb');
+    is_image($im_gfc_c, $im_cfg, "check grey filled with color against base (bits = $bits)");
   }
 }