Fix "Fl_Shared_Image: use of unitialized data" (#216)

- fix issue as proposed
- fix more potential access to uninitialized data issues
- document Fl_Shared_Image::add_handler()
- document typedef Fl_Shared_Image::Fl_Shared_Handler()

2 files changed, 74 insertions, 20 deletions

diff --git a/src/Fl_Shared_Image.cxx b/src/Fl_Shared_Image.cxx
index 5e2399f84..979fe32b0 100644
--- a/src/Fl_Shared_Image.cxx
+++ b/src/Fl_Shared_Image.cxx
@@ -1,7 +1,7 @@
 //
 // Shared image code for the Fast Light Tool Kit (FLTK).
 //
-// Copyright 1998-2017 by Bill Spitzak and others.
+// Copyright 1998-2021 by Bill Spitzak and others.
 //
 // This library is free software. Distribution and use rights are outlined in
 // the file "COPYING" which should have been included with this file.  If this
@@ -249,6 +249,7 @@ void Fl_Shared_Image::release() {
 void Fl_Shared_Image::reload() {
   // Load image from disk...
   int           i;              // Looping var
+  int           count = 0;      // number of bytes read from image header
   FILE          *fp;            // File pointer
   uchar         header[64];     // Buffer for auto-detecting files
   Fl_Image      *img;           // New image
@@ -256,22 +257,23 @@ void Fl_Shared_Image::reload() {
   if (!name_) return;
 
   if ((fp = fl_fopen(name_, "rb")) != NULL) {
-    if (fread(header, 1, sizeof(header), fp)==0) { /* ignore */ }
+    count = fread(header, 1, sizeof(header), fp);
     fclose(fp);
+    if (count == 0)
+      return;
   } else {
     return;
   }
 
   // Load the image as appropriate...
-  if (memcmp(header, "#define", 7) == 0) // XBM file
+  if (count >= 7 && memcmp(header, "#define", 7) == 0) // XBM file
     img = new Fl_XBM_Image(name_);
-  else if (memcmp(header, "/* XPM */", 9) == 0) // XPM file
+  else if (count >= 9 && memcmp(header, "/* XPM */", 9) == 0) // XPM file
     img = new Fl_XPM_Image(name_);
   else {
     // Not a standard format; try an image handler...
     for (i = 0, img = 0; i < num_handlers_; i ++) {
-      img = (handlers_[i])(name_, header, sizeof(header));
-
+      img = (handlers_[i])(name_, header, count);
       if (img) break;
     }
   }
@@ -495,7 +497,19 @@ Fl_Shared_Image *Fl_Shared_Image::get(Fl_RGB_Image *rgb, int own_it)
 
 
 /** Adds a shared image handler, which is basically a test function
-    for adding new formats.
+  for adding new image formats.
+
+  This function will be called when an Fl_Shared_Image is to be loaded
+  (for instance with Fl_Shared_Image::get()) and the image type is not
+  known to FLTK.
+
+  All registered image handlers will be called in the order of registration.
+  You should always call fl_register_images() before adding your own
+  handlers - unless you need to override a known image file type which
+  should be rare.
+
+  \see Fl_Shared_Handler for more information of the function you need
+    to define.
 */
 void Fl_Shared_Image::add_handler(Fl_Shared_Handler f) {
   int                   i;              // Looping var...
diff --git a/src/fl_images_core.cxx b/src/fl_images_core.cxx
index c2d950533..e5487d356 100644
--- a/src/fl_images_core.cxx
+++ b/src/fl_images_core.cxx
@@ -2,6 +2,7 @@
 // FLTK images library core.
 //
 // Copyright 1997-2010 by Easy Software Products.
+// Copyright 2011-2021 by Bill Spitzak and others.
 //
 // This library is free software. Distribution and use rights are outlined in
 // the file "COPYING" which should have been included with this file.  If this
@@ -47,11 +48,13 @@ static Fl_Image *fl_check_images(const char *name, uchar *header, int headerlen)
 
 
 /**
-\brief Register the image formats.
+\brief Register the known image formats.
 
- This function is provided in the fltk_images library and
- registers all of the "extra" image file formats that are not part
- of the core FLTK library.
+  This function is provided in the fltk_images library and
+  registers all of the "extra" image file formats known to FLTK
+  that are not part of the core FLTK library.
+
+  You may add your own image formats with Fl_Shared_Image::add_handler().
 */
 void fl_register_images() {
   Fl_Shared_Image::add_handler(fl_check_images);
@@ -62,49 +65,86 @@ void fl_register_images() {
 //
 // 'fl_check_images()' - Check for a supported image format.
 //
+// returns 0 (NULL) if <headerlen> is less than 6 because:
+//  (1) some of the comparisons would otherwise access undefined data
+//  (2) there's no valid image file with less than 6 bytes
+//
+// Note 1: The number 6 above may be changed if necessary as long as
+//   condition (2) holds.
+//
+// Note 2: The provided buffer <header> MUST NOT be overwritten by any
+//   check function because subsequently called check functions need
+//   the original image header data. <header> should be const!
 
 Fl_Image *                                      // O - Image, if found
 fl_check_images(const char *name,               // I - Filename
                 uchar      *header,             // I - Header data from file
-                int headerlen) {                // I - Amount of data
+                int         headerlen) {        // I - Amount of data in header
+
+  if (headerlen < 6) // not a valid image
+    return 0;
+
+  // GIF
+
   if (memcmp(header, "GIF87a", 6) == 0 ||
       memcmp(header, "GIF89a", 6) == 0) // GIF file
     return new Fl_GIF_Image(name);
 
+  // BMP
+
   if (memcmp(header, "BM", 2) == 0)     // BMP file
     return new Fl_BMP_Image(name);
 
+  // PNM
+
   if (header[0] == 'P' && header[1] >= '1' && header[1] <= '7')
                                         // Portable anymap
     return new Fl_PNM_Image(name);
 
+  // PNG
+
 #ifdef HAVE_LIBPNG
   if (memcmp(header, "\211PNG", 4) == 0)// PNG file
     return new Fl_PNG_Image(name);
 #endif // HAVE_LIBPNG
 
+  // JPEG
+
 #ifdef HAVE_LIBJPEG
   if (memcmp(header, "\377\330\377", 3) == 0 && // Start-of-Image
       header[3] >= 0xc0 && header[3] <= 0xfe)   // APPn .. comment for JPEG file
     return new Fl_JPEG_Image(name);
 #endif // HAVE_LIBJPEG
 
+  // SVG or SVGZ (gzip'ed SVG)
+
 #ifdef FLTK_USE_SVG
-#  if defined(HAVE_LIBZ)
-  if (header[0] == 0x1f && header[1] == 0x8b) { // denotes gzip'ed data
+  uchar header2[64];      // buffer for decompression
+  uchar *buf = header;    // original header data
+  int count = headerlen;  // original header data size
+
+  // Note: variables 'buf' and 'count' may be overwritten subsequently
+  // if the image data is gzip'ed *and* we can decompress the data
+
+# if defined(HAVE_LIBZ)
+  if (header[0] == 0x1f && header[1] == 0x8b) { // gzip'ed data
     int fd = fl_open_ext(name, 1, 0);
     if (fd < 0) return NULL;
-    gzFile gzf =  gzdopen(fd, "r");
+    gzFile gzf = gzdopen(fd, "r");
     if (gzf) {
-      gzread(gzf, header, headerlen);
+      count = gzread(gzf, header2, (int)sizeof(header2));
       gzclose(gzf);
+      buf = header2; // decompressed data
     }
-  }
-#  endif // HAVE_LIBZ
-  if ( (headerlen > 5 && memcmp(header, "<?xml", 5) == 0) ||
-      memcmp(header, "<svg", 4) == 0)
+  } // gzip'ed data
+# endif // HAVE_LIBZ
+
+  if ((count >= 5 && memcmp(buf, "<?xml", 5) == 0) ||
+      (count >= 4 && memcmp(buf, "<svg", 4) == 0))
     return new Fl_SVG_Image(name);
 #endif // FLTK_USE_SVG
 
+  // unknown image format
+
   return 0;
 }