Re: image size limit?

[Chong Yidong]

Ok, I figured out how to do it.  This patch imposes height and width
limits based on the variable max-image-size.  It should cause load_gif
and the other image loading functions to return before the third-party
libraries are called upon to load the image data, so we shouldn't get
memory-overallocation at any point.

I've tested it for png, jpeg, tiff, and gif.  It should work for xbm,
pbm and ghostscript, but I haven't checked.  It doesn't impose limits
on xpm images, because I don't understand the xpm code too well.

If no one objects to this, I'll install it sometime next week.


*** emacs/src/image.c.~1.36.~   2005-10-11 11:09:35.000000000 -0400
--- emacs/src/image.c   2005-10-16 18:10:45.000000000 -0400
***************
*** 1097,1102 ****
--- 1097,1105 ----
                 Image type independent image structures
   ***********************************************************************/
  
+ #define MAX_IMAGE_SIZE 100000
+ EMACS_INT Vmax_image_size;
+ 
  static struct image *make_image P_ ((Lisp_Object spec, unsigned hash));
  static void free_image P_ ((struct frame *f, struct image *img));
  
***************
*** 1708,1713 ****
--- 1711,1722 ----
      if (img->hash == hash && !NILP (Fequal (img->spec, spec)))
        break;
  
+   if (img && img->load_failed_p)
+     {
+       free_image (f, img);
+       img = NULL;
+     }
+ 
    /* If not found, create a new image and cache it.  */
    if (img == NULL)
      {
***************
*** 2992,2998 ****
        expect (XBM_TK_NUMBER);
      }
  
!   if (*width < 0 || *height < 0)
      goto failure;
    else if (data == NULL)
      goto success;
--- 3001,3008 ----
        expect (XBM_TK_NUMBER);
      }
  
!   if (*width <= 0     || *width > Vmax_image_size
!       || *height <= 0 || *height > Vmax_image_size)
      goto failure;
    else if (data == NULL)
      goto success;
***************
*** 5465,5472 ****
        max_color_idx = 255;
      }
  
!   if (width < 0
!       || height < 0
        || (type != PBM_MONO && max_color_idx < 0))
      goto error;
  
--- 5475,5482 ----
        max_color_idx = 255;
      }
  
!   if (width <= 0 || width > Vmax_image_size
!       || height <=0 || height > Vmax_image_size
        || (type != PBM_MONO && max_color_idx < 0))
      goto error;
  
***************
*** 5966,5971 ****
--- 5976,5985 ----
    fn_png_get_IHDR (png_ptr, info_ptr, &width, &height, &bit_depth, 
&color_type,
                   &interlace_type, NULL, NULL);
  
+   if (width <= 0 || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     goto error;
+ 
    /* If image contains simply transparency data, we prefer to
       construct a clipping mask.  */
    if (fn_png_get_valid (png_ptr, info_ptr, PNG_INFO_tRNS))
***************
*** 6726,6731 ****
--- 6740,6752 ----
    width = img->width = cinfo.output_width;
    height = img->height = cinfo.output_height;
  
+   if (width <= 0 || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       longjmp (mgr.setjmp_buffer, 2);
+     }
+ 
    /* Create X image and pixmap.  */
    if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap))
      longjmp (mgr.setjmp_buffer, 2);
***************
*** 7155,7160 ****
--- 7176,7189 ----
       of width x height 32-bit values.  */
    fn_TIFFGetField (tiff, TIFFTAG_IMAGEWIDTH, &width);
    fn_TIFFGetField (tiff, TIFFTAG_IMAGELENGTH, &height);
+   if (width <= 0    || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       UNGCPRO;
+       return 0;
+     }
+ 
    buf = (uint32 *) xmalloc (width * height * sizeof *buf);
  
    rc = fn_TIFFReadRGBAImage (tiff, width, height, buf, 0);
***************
*** 7459,7464 ****
--- 7488,7504 ----
        }
      }
  
+   /* Abort if the stated image size is too big, before reading entire
+      contents. */
+   if (gif->SWidth <= 0 || gif->SWidth > Vmax_image_size
+       || gif->SHeight <=0 || gif->SHeight > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       fn_DGifCloseFile (gif);
+       UNGCPRO;
+       return 0;
+     }
+ 
    /* Read entire contents.  */
    rc = fn_DGifSlurp (gif);
    if (rc == GIF_ERROR)
***************
*** 7492,7497 ****
--- 7532,7546 ----
                              max (gif->Image.Top + gif->Image.Height,
                                   image_top + image_height));
  
+   if (width <= 0    || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       fn_DGifCloseFile (gif);
+       UNGCPRO;
+       return 0;
+     }
+ 
    /* Create the X image and pixmap.  */
    if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap))
      {
***************
*** 7944,7949 ****
--- 7993,8005 ----
    in_height = XFASTINT (pt_height) / 72.0;
    img->height = in_height * FRAME_X_DISPLAY_INFO (f)->resy;
  
+   if (img->width <= 0    || img->width > Vmax_image_size
+       || img->height <=0 || img->height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       return 0;
+     }
+ 
    /* Create the pixmap.  */
    xassert (img->pixmap == NO_PIXMAP);
  
***************
*** 8217,8222 ****
--- 8273,8286 ----
    Vimage_library_alist = Qnil;
    Fput (intern ("image-library-alist"), Qrisky_local_variable, Qt);
  
+   DEFVAR_INT ("max-image-size", &Vmax_image_size,
+     doc: /* Maximum width and height, in pixels, that an image can have.
+ 
+ Emacs will not load an image into memory if its width or height, in
+ pixels, is larger than this value.  Additional limits may be imposed
+ by the external libraries that Emacs uses to read the images. */);
+   Vmax_image_size = MAX_IMAGE_SIZE;
+ 
    Vimage_type_cache = Qnil;
    staticpro (&Vimage_type_cache);