- the BMP reader now validates the bfOffBits value from the BMP header

  and skips to that offset before reading image data.  Previously this
  value was read but otherwise ignored.

diff --git a/Changes b/Changes
index a8096e37f83aae2d8dce801ad1c75b4d0ee9a753..1a73579785dca35bc4bec0d30f2a84148ec7443f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -908,6 +908,9 @@ Revision history for Perl extension Imager.
 - added tools/imager to the distribution.  This is still very 
   experimental and untested.  Patches welcome, if you write tests to go
   with them.
+- the BMP reader now validates the bfOffBits value from the BMP header
+  and skips to that offset before reading image data.  Previously this
+  value was read but otherwise ignored.
 
 =================================================================
 

diff --git a/MANIFEST b/MANIFEST
index e1a0d04c19cb4e5ad6df115dbfee895ad0a94dc1..a89cb887c4f2ba48e0a4799d07d6d7fd2eceef74 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -203,6 +203,14 @@ testimg/simple.pbm
 testimg/test_gimp_pal   A simple GIMP palette file
 testimg/trimgdesc.gif
 testimg/trmiddesc.gif
+testimg/winrgb2.bmp  1-bit bmp base
+testimg/winrgb24.bmp 24-bit bmp base
+testimg/winrgb24off.bmp 24-bit bmp with image data offset from header
+testimg/winrgb2off.bmp  1-bit bmp with image data offset from header
+testimg/winrgb4.bmp  4-bit bmp base
+testimg/winrgb4off.bmp  4-bit bmp with image data offset from header
+testimg/winrgb8.bmp  8-bit bmp base
+testimg/winrgb8off.bmp  8-bit bmp with image data offset from header
 tga.c           Reading and writing Targa files
 tiff.c
 tools/imager    Command-line tool for imager (experimental)

diff --git a/TODO b/TODO
index 42b56bbca9f035e4ca030083fbb75124db9bfd9a..a3c7fffd2dcc9f9adba56d322a3aa8c763610fec 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,5 +1,27 @@
                          *** TODO ***
 
+PRE-0.44:
+- set i_format for every file type on read and test for it:
+  - bmp - done
+  - jpeg - done
+  - tiff
+  - tga
+  - rgb
+  - png - done
+  - gif
+  - pnm
+- check each file reader for possible integer overflows
+  - bmp
+  - tiff
+  - tga
+  - rgb
+  - png
+  - gif
+  - pnm
+- check bmp code uses image data offset correctly - done
+- check quant code for integer overflows
+- check for old URLs (umich and imager.perl.org/~addi/...)
+
 Iolayer:
 - Add scalar/mmap to iolayer
 - Add close() code to iolayer for fakeseek sources.

diff --git a/bmp.c b/bmp.c
index 4e103272a9646c10fbd7e3c20dcd62d2ff0082c6..d155fd885450b23ad4cb7004f40c201ba6b41e34 100644 (file)
--- a/bmp.c
+++ b/bmp.c
@@ -45,13 +45,14 @@ static int write_8bit_data(io_glue *ig, i_img *im);
 static int write_24bit_data(io_glue *ig, i_img *im);
 static int read_bmp_pal(io_glue *ig, i_img *im, int count);
 static i_img *read_1bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-                            int compression);
+                            int compression, long offbits);
 static i_img *read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-                            int compression);
+                            int compression, long offbits);
 static i_img *read_8bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-                            int compression);
+                            int compression, long offbits);
 static i_img *read_direct_bmp(io_glue *ig, int xsize, int ysize, 
-                              int bit_count, int clr_used, int compression);
+                              int bit_count, int clr_used, int compression,
+                              long offbits);
 
 /* 
 =item i_writebmp_wiol(im, io_glue)
@@ -134,21 +135,22 @@ i_readbmp_wiol(io_glue *ig) {
   
   switch (bit_count) {
   case 1:
-    im = read_1bit_bmp(ig, xsize, ysize, clr_used, compression);
+    im = read_1bit_bmp(ig, xsize, ysize, clr_used, compression, offbits);
     break;
 
   case 4:
-    im = read_4bit_bmp(ig, xsize, ysize, clr_used, compression);
+    im = read_4bit_bmp(ig, xsize, ysize, clr_used, compression, offbits);
     break;
 
   case 8:
-    im = read_8bit_bmp(ig, xsize, ysize, clr_used, compression);
+    im = read_8bit_bmp(ig, xsize, ysize, clr_used, compression, offbits);
     break;
 
   case 32:
   case 24:
   case 16:
-    im = read_direct_bmp(ig, xsize, ysize, bit_count, clr_used, compression);
+    im = read_direct_bmp(ig, xsize, ysize, bit_count, clr_used, compression,
+                         offbits);
     break;
 
   default:
@@ -614,7 +616,7 @@ read_bmp_pal(io_glue *ig, i_img *im, int count) {
 }
 
 /*
-=item read_1bit_bmp(ig, xsize, ysize, clr_used)
+=item read_1bit_bmp(ig, xsize, ysize, clr_used, compression, offbits)
 
 Reads in the palette and image data for a 1-bit/pixel image.
 
@@ -624,7 +626,7 @@ Returns the image or NULL.
 */
 static i_img *
 read_1bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-              int compression) {
+              int compression, long offbits) {
   i_img *im;
   int x, y, lasty, yinc;
   i_palidx *line, *p;
@@ -632,6 +634,7 @@ read_1bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
   int line_size = (xsize + 7)/8;
   int byte, bit;
   unsigned char *in;
+  long base_offset;
 
   if (compression != BI_RGB) {
     i_push_errorf(0, "unknown 1-bit BMP compression (%d)", compression);
@@ -658,6 +661,13 @@ read_1bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     i_push_errorf(0, "out of range colors used (%d)", clr_used);
     return NULL;
   }
+
+  base_offset = FILEHEAD_SIZE + INFOHEAD_SIZE + clr_used * 4;
+  if (offbits < base_offset) {
+    i_push_errorf(0, "image data offset too small (%ld)", offbits);
+    return NULL;
+  }
+
   im = i_img_pal_new(xsize, ysize, 3, 256);
   if (!im)
     return NULL;
@@ -665,6 +675,20 @@ read_1bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     i_img_destroy(im);
     return NULL;
   }
+
+  if (offbits > base_offset) {
+    /* this will be slow if the offset is large, but that should be
+       rare */
+    char buffer;
+    while (base_offset < offbits) {
+      if (ig->readcb(ig, &buffer, 1) != 1) {
+        i_img_destroy(im);
+        i_push_error(0, "failed skipping to image data offset");
+        return NULL;
+      }
+      ++base_offset;
+    }
+  }
   
   i_tags_add(&im->tags, "bmp_compression_name", 0, "BI_RGB", -1, 0);
 
@@ -712,7 +736,7 @@ point.
 */
 static i_img *
 read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-              int compression) {
+              int compression, long offbits) {
   i_img *im;
   int x, y, lasty, yinc;
   i_palidx *line, *p;
@@ -720,6 +744,7 @@ read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
   int line_size = (xsize + 1)/2;
   unsigned char *in;
   int size, i;
+  long base_offset;
 
   line_size = (line_size+3) / 4 * 4;
 
@@ -743,6 +768,12 @@ read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     return NULL;
   }
 
+  base_offset = FILEHEAD_SIZE + INFOHEAD_SIZE + clr_used * 4;
+  if (offbits < base_offset) {
+    i_push_errorf(0, "image data offset too small (%ld)", offbits);
+    return NULL;
+  }
+
   im = i_img_pal_new(xsize, ysize, 3, 256);
   if (!im) /* error should have been pushed already */
     return NULL;
@@ -751,6 +782,20 @@ read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     return NULL;
   }
 
+  if (offbits > base_offset) {
+    /* this will be slow if the offset is large, but that should be
+       rare */
+    char buffer;
+    while (base_offset < offbits) {
+      if (ig->readcb(ig, &buffer, 1) != 1) {
+        i_img_destroy(im);
+        i_push_error(0, "failed skipping to image data offset");
+        return NULL;
+      }
+      ++base_offset;
+    }
+  }
+  
   if (line_size < 260)
     packed = mymalloc(260);
   else
@@ -873,12 +918,13 @@ Returns the image or NULL.
 */
 static i_img *
 read_8bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, 
-              int compression) {
+              int compression, long offbits) {
   i_img *im;
   int x, y, lasty, yinc;
   i_palidx *line, *p;
   int line_size = xsize;
   unsigned char *in;
+  long base_offset;
 
   line_size = (line_size+3) / 4 * 4;
 
@@ -901,6 +947,12 @@ read_8bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     return NULL;
   }
 
+  base_offset = FILEHEAD_SIZE + INFOHEAD_SIZE + clr_used * 4;
+  if (offbits < base_offset) {
+    i_push_errorf(0, "image data offset too small (%ld)", offbits);
+    return NULL;
+  }
+
   im = i_img_pal_new(xsize, ysize, 3, 256);
   if (!im)
     return NULL;
@@ -909,6 +961,20 @@ read_8bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used,
     return NULL;
   }
 
+  if (offbits > base_offset) {
+    /* this will be slow if the offset is large, but that should be
+       rare */
+    char buffer;
+    while (base_offset < offbits) {
+      if (ig->readcb(ig, &buffer, 1) != 1) {
+        i_img_destroy(im);
+        i_push_error(0, "failed skipping to image data offset");
+        return NULL;
+      }
+      ++base_offset;
+    }
+  }
+  
   line = mymalloc(line_size);
   if (compression == BI_RGB) {
     i_tags_add(&im->tags, "bmp_compression_name", 0, "BI_RGB", -1, 0);
@@ -1023,7 +1089,7 @@ Returns the image or NULL.
 */
 static i_img *
 read_direct_bmp(io_glue *ig, int xsize, int ysize, int bit_count, 
-                int clr_used, int compression) {
+                int clr_used, int compression, long offbits) {
   i_img *im;
   int x, y, lasty, yinc;
   i_color *line, *p;
@@ -1037,6 +1103,7 @@ read_direct_bmp(io_glue *ig, int xsize, int ysize, int bit_count,
   char junk[4];
   const char *compression_name;
   int bytes;
+  long base_offset = FILEHEAD_SIZE + INFOHEAD_SIZE;
   
   unpack_code[0] = *("v3V"+pix_size-2);
   unpack_code[1] = '\0';
@@ -1067,6 +1134,7 @@ read_direct_bmp(io_glue *ig, int xsize, int ysize, int bit_count,
         i_push_error(0, "skipping colors");
         return 0;
       }
+      base_offset += 4;
     }
   }
   else if (compression == BI_BITFIELDS) {
@@ -1087,12 +1155,27 @@ read_direct_bmp(io_glue *ig, int xsize, int ysize, int bit_count,
       }
       masks.shifts[i] = pos - 8;
     }
+    base_offset += 4 * 4;
   }
   else {
     i_push_errorf(0, "unknown 24-bit BMP compression (%d)", compression);
     return NULL;
   }
 
+  if (offbits > base_offset) {
+    /* this will be slow if the offset is large, but that should be
+       rare */
+    char buffer;
+    while (base_offset < offbits) {
+      if (ig->readcb(ig, &buffer, 1) != 1) {
+        i_img_destroy(im);
+        i_push_error(0, "failed skipping to image data offset");
+        return NULL;
+      }
+      ++base_offset;
+    }
+  }
+  
   im = i_img_empty(NULL, xsize, ysize);
   if (!im)
     return NULL;
@@ -1157,5 +1240,7 @@ Doesn't handle OS/2 bitmaps.
 
 BI_BITFIELDS compression hasn't been tested (I need an image).
 
+The header handling for paletted images needs to be refactored
+
 =cut
 */

diff --git a/t/t107bmp.t b/t/t107bmp.t
index 62d8d9c73cf81d1e24b2cb0f69460bf696498320..ba1981cc2187b3155206b78bdab2eb1254821172 100644 (file)
--- a/t/t107bmp.t
+++ b/t/t107bmp.t
@@ -1,9 +1,9 @@
 #!perl -w
-print "1..62\n";
+print "1..74\n";
 use Imager qw(:all);
 use strict;
 init_log("testout/t107bmp.log",1);
-require 't/testtools.pl';
+BEGIN { require 't/testtools.pl'; } # BEGIN to apply prototypes
 
 my $base_diff = 0;
 # if you change this make sure you generate new compressed versions
@@ -152,6 +152,41 @@ for my $test (@tests) {
     $test_num += 2;
   }
 }
+
+# previously we didn't seek to the offbits position before reading
+# the image data, check we handle it correctly
+# in each case the first is an original image with a given number of
+# bits and the second is the same file with data inserted before the
+# image bits and the offset modified to suit
+my @comp =
+  (
+   [ 'winrgb2.bmp', 'winrgb2off.bmp', 1 ],
+   [ 'winrgb4.bmp', 'winrgb4off.bmp', 4 ],
+   [ 'winrgb8.bmp', 'winrgb8off.bmp', 8 ],
+   [ 'winrgb24.bmp', 'winrgb24off.bmp', 24 ],
+  );
+
+for my $comp (@comp) {
+  my ($base_file, $off_file, $bits) = @$comp;
+
+  my $base_im = Imager->new;
+  my $got_base = 
+    okn($test_num++, $base_im->read(file=>"testimg/$base_file"),
+        "read original")
+      or print "# ",$base_im->errstr,"\n";
+  my $off_im = Imager->new;
+  my $got_off =
+    okn($test_num++, $off_im->read(file=>"testimg/$off_file"),
+        "read offset file")
+      or print "# ",$off_im->errstr,"\n";
+  if ($got_base && $got_off) {
+    okn($test_num++, !i_img_diff($base_im->{IMG}, $off_im->{IMG}), 
+        "compare base and offset image ($bits bits)");
+  }
+  else {
+    skipn($test_num++, 1, "missed one file");
+  }
+}
                               
 sub write_test {
   my ($test_num, $im, $filename) = @_;

diff --git a/tags.c b/tags.c
index f8440808bfb5840ceee84630ecd027fbd061b27b..09880667eea1a90771a69f68057a12b2d362153c 100644 (file)
--- a/tags.c
+++ b/tags.c
@@ -125,6 +125,8 @@ int i_tags_add(i_img_tags *tags, char const *name, int code, char const *data,
     strcpy(work.name, name);
   }
   if (data) {
+    if (size == -1)
+      size = strlen(data);
     work.data = mymalloc(size+1);
     if (!work.data) {
       if (work.name) myfree(work.name);

diff --git a/testimg/winrgb24off.bmp b/testimg/winrgb24off.bmp
new file mode 100644 (file)
index 0000000..29208bf
Binary files /dev/null and b/testimg/winrgb24off.bmp differ

diff --git a/testimg/winrgb2off.bmp b/testimg/winrgb2off.bmp
new file mode 100644 (file)
index 0000000..213d9ad
Binary files /dev/null and b/testimg/winrgb2off.bmp differ

diff --git a/testimg/winrgb4off.bmp b/testimg/winrgb4off.bmp
new file mode 100644 (file)
index 0000000..a560d41
Binary files /dev/null and b/testimg/winrgb4off.bmp differ

diff --git a/testimg/winrgb8off.bmp b/testimg/winrgb8off.bmp
new file mode 100644 (file)
index 0000000..f537cc0
Binary files /dev/null and b/testimg/winrgb8off.bmp differ