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()

FL/Fl_Shared_Image.H

@@ -1,7 +1,7 @@
 //
 // Shared image header file 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
@@ -23,11 +23,54 @@
 #  include "Fl_Image.H"
 
 
-// Test function for adding new formats
-typedef Fl_Image *(*Fl_Shared_Handler)(const char *name, uchar *header,
+/** Test function (typedef) for adding new shared image formats.
+
+  This defines the function type you can use to add a handler for unknown
+  image formats that can be opened and loaded as an Fl_Shared_Image.
+
+  fl_register_images() adds all image formats known to FLTK.
+  Call Fl_Shared_Image::add_handler() to add your own check function to
+  the list of known image formats.
+
+  Your function will be passed the filename (\p name), some \p header
+  bytes already read from the image file and the size \p headerlen of the
+  data read. The max value of size is implementation dependent. If your
+  handler function needs to check more bytes you must open the image file
+  yourself.
+
+  The provided buffer \p header must not be overwritten.
+
+  If your handler function can identify the file type you must open the
+  file and return a valid Fl_Image or derived type, otherwise you must
+  return \c NULL.
+  Example:
+  \code
+    static Fl_Image *check_my_image(const char *name,
+                                    uchar *header,
+                                    int headerlen) {
+      // (test image type using header and headerlen)
+      if (known) {
+        // (load image data from file \p name)
+        return new Fl_RGB_Image(data, ...);
+      } else
+        return 0;
+    }
+    // add your handler:
+    Fl_Shared_Image::add_handler(check_my_image);
+  \endcode
+
+  \param[in]    name        filename to be checked and opened if applicable
+  \param[in]    header      portion of the file that has already been read
+  \param[in]    headerlen   length of provided \p header data
+
+  \returns      valid Fl_Image or \c NULL.
+
+  \see Fl_Shared_Image::add_handler()
+*/
+typedef Fl_Image *(*Fl_Shared_Handler)(const char *name,
+                                       uchar *header,
                                        int headerlen);
 
-// Shared images class.
 /**
   This class supports caching, loading, and drawing of image files.
 
@@ -42,6 +85,7 @@ typedef Fl_Image *(*Fl_Shared_Handler)(const char *name, uchar *header,
   A refcount is used to determine if a released image is to be destroyed
   with delete.
 
+  \see fl_register_image()
   \see Fl_Shared_Image::get()
   \see Fl_Shared_Image::find()
   \see Fl_Shared_Image::release()

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...

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;
 }