Re: image size limit?
RMS: But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. Stefan: All this to say that I think choosing the maximum image size based on display-pixel-width and display-pixel-height would be preferable than using frame size. If you use image slicing, you can in principle show a small area of a much larger image. I don't see how that relates to the dimensions of the frame or display. But it definitely sounds better to scale according to display size rather than frame size (but round up to minimum size e.g. 4096x4096). -- Kim F. Storm [EMAIL PROTECTED] http://www.cua.dk ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
From: Chong Yidong [EMAIL PROTECTED] Date: Mon, 17 Oct 2005 17:56:39 -0400 Cc: emacs-devel@gnu.org But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. What frame should we then use? The selected frame? That's not really logical, the frame that issues the call to load an image (which is where we check the image size) may not be the frame that ends up displaying it. How about the using display size in pixels (display-pixel-height and display-pixel-width)? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
[EMAIL PROTECTED] (Kim F. Storm) writes: RMS: But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. Stefan: All this to say that I think choosing the maximum image size based on display-pixel-width and display-pixel-height would be preferable than using frame size. If you use image slicing, you can in principle show a small area of a much larger image. I don't see how that relates to the dimensions of the frame or display. But it definitely sounds better to scale according to display size rather than frame size (but round up to minimum size e.g. 4096x4096). It sounds to me like the limits should be configurable, with a somewhat conservative default. Applications where larger dimensions might be appropriate (image viewers with provisions for panning, i.e.) can allow them in their own buffers using buffer-local settings of the variables limiting the size. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
David Kastrup [EMAIL PROTECTED] writes: But it definitely sounds better to scale according to display size rather than frame size (but round up to minimum size e.g. 4096x4096). It sounds to me like the limits should be configurable, with a somewhat conservative default. Applications where larger dimensions might be appropriate (image viewers with provisions for panning, i.e.) can allow them in their own buffers using buffer-local settings of the variables limiting the size. Scaling according to display size isn't entirely problem-free either, because Emacs frames can be put on different X displays ;-) I think scaling according to frame size is not a great solution, but it's OK because of the way image loading works. If the frame is initially too small, Emacs won't load the image. But each time you increase the size of the frame, Emacs checks again, and loads the image if the size relative to the frame is now OK. Once an image is loaded, it's put in the image cache, so it will always be displayed regardless of the frame size. The max-image-size code is checked into CVS. I've set a conservative value of 6.0 times the frame width and height. After all, the original rationale for this feature is to avoid over-allocating memory in case a malicious image demands a gigantic image widths/heights of millions of pixels. In ordinary situations, it shouldn't get in the way of the user. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
David Kastrup [EMAIL PROTECTED] writes: It sounds to me like the limits should be configurable, with a somewhat conservative default. Applications where larger dimensions might be appropriate (image viewers with provisions for panning, i.e.) can allow them in their own buffers using buffer-local settings of the variables limiting the size. I think it makes sense for some purposes to allow max image size to be specified in absolute pixels. Here is a patch to do that: *** image.c 19 Oct 2005 10:20:42 +0200 1.37 --- image.c 19 Oct 2005 11:50:56 +0200 *** *** 1163,1179 int width; int height; { ! if (width = 0 || height =0) ! return 0; ! if (FLOATP (Vmax_image_size) f !((width (int)(XFLOAT_DATA (Vmax_image_size) !* FRAME_PIXEL_WIDTH (f))) ! || (height (int)(XFLOAT_DATA (Vmax_image_size) !* FRAME_PIXEL_HEIGHT (f) return 0; ! return 1; } /* Prepare image IMG for display on frame F. Must be called before --- 1163,1191 int width; int height; { ! int w, h; ! if (width = 0 || height = 0) return 0; ! if (INTEGERP (Vmax_image_size)) ! w = h = XINT (Vmax_image_size); ! else if (FLOATP (Vmax_image_size)) ! { ! if (f != NULL) ! { ! w = FRAME_PIXEL_WIDTH (f); ! h = FRAME_PIXEL_HEIGHT (f); ! } ! else ! w = h = 1024; /* Arbitrary size for unknown frame. */ ! w = (int) (XFLOAT_DATA (Vmax_image_size) * w); ! h = (int) (XFLOAT_DATA (Vmax_image_size) * h); ! } ! else ! return 1; ! ! return (width = w height = h); } /* Prepare image IMG for display on frame F. Must be called before *** *** 8289,8300 Fput (intern (image-library-alist), Qrisky_local_variable, Qt); DEFVAR_LISP (max-image-size, Vmax_image_size, ! doc: /* Maximum size of an image, relative to the selected frame. ! This is a floating point number that is multiplied by the width and height of the selected frame, to give the maximum width and height for ! images. Emacs will not load an image into memory if its width or ! height exceeds this limit. */); Vmax_image_size = make_float (MAX_IMAGE_SIZE); Vimage_type_cache = Qnil; --- 8301,8319 Fput (intern (image-library-alist), Qrisky_local_variable, Qt); DEFVAR_LISP (max-image-size, Vmax_image_size, ! doc: /* Maximum size of images. ! Emacs will not load an image into memory if its pixel width or ! pixel height exceeds this limit. ! ! If the value is an integer it specifies the absolute maximum pixel ! width and pixel height for images. ! If the value is a floating point number it specifies a limit relative ! to the selected frame, i.e. the value is multiplied by the width and height of the selected frame, to give the maximum width and height for ! images. ! ! Otherwise, no limit is placed on images. */); Vmax_image_size = make_float (MAX_IMAGE_SIZE); Vimage_type_cache = Qnil; *** display.texi19 Oct 2005 10:20:40 +0200 1.191 --- display.texi19 Oct 2005 13:12:43 +0200 *** *** 4061,4070 @defvar max-image-size @tindex max-image-size This variable is used to define the maximum size of image that Emacs ! will load. If its value is a floating point number, that number is ! multiplied by the width and height of the selected frame, in pixels, ! to give the maximum image width and height. Emacs will refuse to load ! and display any image that is larger than this. The purpose of this variable is to prevent unreasonably large images from accidentally being loaded into Emacs. It only takes effect the --- 4061,4074 @defvar max-image-size @tindex max-image-size This variable is used to define the maximum size of image that Emacs ! will load. Emacs will refuse to load (and display) any image that is ! larger than this limit. ! ! If its value is an integer, it specifies the absolute maximum width ! and height, in pixels, for images. If its value is a floating point ! number, that number is multiplied by the width and height of the ! selected frame, in pixels, to give the maximum image width and height. ! Otherwise, no limit is placed on images. The purpose of this variable is to prevent unreasonably large images from accidentally being loaded into Emacs. It only takes effect the -- Kim F. Storm [EMAIL PROTECTED] http://www.cua.dk ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
2005/10/18, Stefan Monnier [EMAIL PROTECTED]: My Emacs frames are typically 80 columns of 13x6, i.e. about 500 pixel wide, but I often (at least once a week) watch digital photos in those frames and more often than not those picteures are 1600 or 2048 pixel wide. Yeah, me too -- in many cases I just wanna check do a quick check, and seeing just the upper-left corner is good enough, so I do this with pictures up to about 3840 x 3072 pixels (that's triple my display size in each dimension). Emacs scrolling actually works reasonably well if you only care about the left edge... :-) I think it's a good idea to have a limit, just make it rather bigger than any common picture size, e.g., 10,000 x 10,000. Limiting it to some multiple of the display size seems like pointless extra work -- Emacs on a modern machine is quite capable of handling even pretty big pictures, it's just the _insane_ sizes from corrupted or malicious data that cause problems. -Miles -- Do not taunt Happy Fun Ball. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
The max-image-size code is checked into CVS. I've set a conservative value of 6.0 times the frame width and height. I am concerned that that is too large. On large displays, an Emacs frame could be 1000 pixels in each dimension. 6000 pixels on each side is 36 million pixels. Isn't that enough to cause you a lot of trouble? It seems to me it should be 3.0, not 6.0. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
I think it makes sense for some purposes to allow max image size to be specified in absolute pixels. Here is a patch to do that: That is ok. But instead of this text ! If its value is an integer, it specifies the absolute maximum width ! and height, in pixels, for images. If its value is a floating point ! number, that number is multiplied by the width and height of the ! selected frame, in pixels, to give the maximum image width and height. ! Otherwise, no limit is placed on images. it would be clearer to write this: ! If the value is an integer, it directly specifies the maximum ! image height and width, measured in pixels. If it is a floating ! point number, it specifies the maximum image height and width ! as a ratio to the frame height and width. If the value is ! non-numeric, there is no explicit limit on the size of images. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
Richard M. Stallman [EMAIL PROTECTED] writes: it would be clearer to write this: ! If the value is an integer, it directly specifies the maximum ! image height and width, measured in pixels. If it is a floating ! point number, it specifies the maximum image height and width ! as a ratio to the frame height and width. If the value is ! non-numeric, there is no explicit limit on the size of images. Much better indeed. I have committed the changes. -- Kim F. Storm [EMAIL PROTECTED] http://www.cua.dk ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
Yeah, me too -- in many cases I just wanna check do a quick check, and seeing just the upper-left corner is good enough, so I do this with pictures up to about 3840 x 3072 pixels (that's triple my display size in each dimension). That is surely an unusual usage. I think it is ok if this requires you to set the value of max-image-size. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. My Emacs frames are typically 80 columns of 13x6, i.e. about 500 pixel wide, but I often (at least once a week) watch digital photos in those frames and more often than not those picteures are 1600 or 2048 pixel wide. Admittedly, it's not very convenient to watch those pictures within Emacs right now (the vertical/horizontal scrolling is far from perfect), but often I end up temporarilly maximizing the frame (after displaying the picture) to 1600 pixel wide. All this to say that I think choosing the maximum image size based on display-pixel-width and display-pixel-height would be preferable than using frame size. Stefan ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
+ #define MAX_IMAGE_SIZE 10 + EMACS_INT Vmax_image_size; 10 as a limit of linear dimension is too large. A square image 10-1 on each side will be 10 billion pixels, far too large to fit in a 32 bit machine. Such a limit should be more like 3000. (That would allow almost 10 million pixels, which is still very big.) But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. The error messages should say Image too large rather than Invalid image size. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. What frame should we then use? The selected frame? That's not really logical, the frame that issues the call to load an image (which is where we check the image size) may not be the frame that ends up displaying it. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
But I don't think this limit should be absolute. I think it should be specified as a multiple of the frame height and width, and it should be given as a floating point number. I'd suggest 2.0 as the default for this ratio. What frame should we then use? The selected frame? The frame that the image is to be displayed in. That's not really logical, the frame that issues the call to load an image (which is where we check the image size) may not be the frame that ends up displaying it. How could they be different? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
As for libpng, the png_set_user_limits() function was only added in version 1.0.16rc1, from 2004, so if we use that we'll lose compatibility with older versions. I think it is ok for the new Emacs to use a feature of libpng that is one year old. Would you like to write the code? Perhaps configure should verify that that entry point exists, and call it only if it is available. At the same time, we implement image width and height limits in x_create_x_image_and_pixmap, to deal with malicious images that specify gigantic width and height sizes, even though the file size isn't that big. Will that succeed in handling the problem case we got? It will avoid allocating too much memory in Emacs. Whether libungif tries to allocate too much memory prior to this, depends on the internal implementation of libungif. Worst that could happen is that libungif believes the invalid width and height data, tries to malloc a big chunk of memory, malloc fails, and DGifSlurp() frees the memory and returns with an error, which we catch. I am not sure that is the worst that can happen, but if it does happen, I agree it is better than what happens now. So would you like to implement that? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
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.0 -0400 --- emacs/src/image.c 2005-10-16 18:10:45.0 -0400 *** *** 1097,1102 --- 1097,1105 Image type independent image structures ***/ + #define MAX_IMAGE_SIZE 10 + 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 =
Re: image size limit?
Here's another idea. We impose a limit on the size of the image file that Emacs is willing to read, say 50 megabytes. This should be easy to implement and independent of image type. That might be a good idea, but it won't address this problem. In this problem case, the file itself is not too large, but the image size is too large due to invalid data. At the same time, we implement image width and height limits in x_create_x_image_and_pixmap, to deal with malicious images that specify gigantic width and height sizes, even though the file size isn't that big. Will that succeed in handling the problem case we got? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
At the same time, we implement image width and height limits in x_create_x_image_and_pixmap, to deal with malicious images that specify gigantic width and height sizes, even though the file size isn't that big. Will that succeed in handling the problem case we got? It will avoid allocating too much memory in Emacs. Whether libungif tries to allocate too much memory prior to this, depends on the internal implementation of libungif. Worst that could happen is that libungif believes the invalid width and height data, tries to malloc a big chunk of memory, malloc fails, and DGifSlurp() frees the memory and returns with an error, which we catch. I can't find any libungif function for controlling how much memory libungif uses. As for libpng, the png_set_user_limits() function was only added in version 1.0.16rc1, from 2004, so if we use that we'll lose compatibility with older versions. I don't think it's worth delaying the release for this. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
For some image types, it is a simple matter of passing to the library the limits Emacs wants to set. For libpng, we could use png_set_user_limits() to set our maximum width/height. (libpng also has a built-in default limit of 1 million pixels in both dimensions.) For other libraries, the application can set the maximum amount of memory to allocate, see for example libjpeg's `max_memory_to_use' and `max_alloc_chunk'. Getting things right for all image types will probably be tricky... If we handle them one by one, it won't be terribly tricky. We could have one limit for memory size of an image, and another limit for the width and height as a ratio to those of the window. Then for each image type it would use whichever of those can be used. Would you like to implement this for some image types for which it is easy to do? Then we could ask the developers of the other libraries to add similar features. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
One place where we can set a limit is in x_create_x_image_and_pixmap, where we malloc a pixmap to store the image contents. The data supplied to us by the external library is copied into this pixmap. We could signal an error if width and height are too large. However, this seems like closing the barn door after the horses have left -- the external library will already have allocated a big chunk of memory. Will it free that memory if Emacs decides to abort the operation? If so, I think that still counts as a solution. If not, I think it is a bug in the library--so we should ask them to fix it. Meanwhile, if these libraries do not have the feature of limiting the memory they can use, I think they ought to have it. That is a necessary part of defending against invalid data. A nonsensical image that swallows all of memory is the equivalent of a denial-of-service attack. Good apps defend against that, and good libraries should be designed to help apps defend against that. Would you like to write to the developers of these libraries, asking them nicely to add such a feature? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
Re: image size limit?
For some image types, it is a simple matter of passing to the library the limits Emacs wants to set. For libpng, we could use png_set_user_limits() to set our maximum width/height. (libpng also has a built-in default limit of 1 million pixels in both dimensions.) For other libraries, the application can set the maximum amount of memory to allocate, see for example libjpeg's `max_memory_to_use' and `max_alloc_chunk'. Getting things right for all image types will probably be tricky... -- Romain Francoise [EMAIL PROTECTED] | I just thought I'd go out it's a miracle -- http://orebokech.com/ | with a little bit more style. ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel
image size limit?
Can we get this discussion going again? From FOR-RELEASE: Put a max-limit on the size of images, e.g. based on the display size. This is to avoid allocating insane amounts of memory due to bogus image size specifications. Note: rather than clipping images that are too big (this may be non-trivial to do correctly in all cases -- and thus non-trivial to test), it may be better just to avoid displaying such images for emacs 22. I am not an expert, but from looking through image.c, it seems that there is only so much we can do to prevent memory over-allocation problems. Generally, we have to call an external library to load an image file, which does its own memory management thing. By the time we know the width and height, the external library has already loaded the file into memory (or signalled an error). For example, gif_load looks something like this: gif = (GifFileType *)fn_DGifOpenFileName (SDATA (file)); /* Read entire contents. */ rc = fn_DGifSlurp (gif); ... image_top = gif-SavedImages[ino].ImageDesc.Top; ... x_create_x_image_and_pixmap (f, width, height, ...); We have no control over what DGifSlurp does. One place where we can set a limit is in x_create_x_image_and_pixmap, where we malloc a pixmap to store the image contents. The data supplied to us by the external library is copied into this pixmap. We could signal an error if width and height are too large. However, this seems like closing the barn door after the horses have left -- the external library will already have allocated a big chunk of memory. Thoughts? ___ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel