From ce2955ec71284d12259b16fc7cf472ab2d16f83b Mon Sep 17 00:00:00 2001 From: hugovk Date: Wed, 14 May 2014 18:04:18 +0300 Subject: [PATCH 01/11] Throw an exception when an opened image is larger than an arbitrary limit --- PIL/Image.py | 68 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/PIL/Image.py b/PIL/Image.py index 333397701..e33454d90 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -35,6 +35,12 @@ class _imaging_not_installed: def __getattr__(self, id): raise ImportError("The _imaging C module is not installed") + +class ImageIsTooBigError(Exception): + pass + +ARBITARY_LARGE_LIMIT = 6000 * 6000 - 1 # FIXME: Pick sensible limit + try: # give Tk a chance to set up the environment, in case we're # using an _imaging module linked against libtcl/libtk (use @@ -101,7 +107,7 @@ import collections import numbers # works everywhere, win for pypy, not cpython -USE_CFFI_ACCESS = hasattr(sys, 'pypy_version_info') +USE_CFFI_ACCESS = hasattr(sys, 'pypy_version_info') try: import cffi HAS_CFFI=True @@ -233,7 +239,7 @@ _MODE_CONV = { "CMYK": ('|u1', 4), "YCbCr": ('|u1', 3), "LAB": ('|u1', 3), # UNDONE - unsigned |u1i1i1 - # I;16 == I;16L, and I;32 == I;32L + # I;16 == I;16L, and I;32 == I;32L "I;16": ('u2', None), "I;16L": (' 8bit images. + # a gamma function point transform on > 8bit images. scale, offset = _getscaleoffset(lut) return self._new(self.im.point_transform(scale, offset)) # for other modes, convert the function to a table @@ -1420,8 +1426,8 @@ class Image: self._copy() self.pyaccess = None self.load() - - if self.pyaccess: + + if self.pyaccess: return self.pyaccess.putpixel(xy,value) return self.im.putpixel(xy, value) @@ -2100,7 +2106,18 @@ _fromarray_typemap[((1, 1), _ENDIAN + "i4")] = ("I", "I") _fromarray_typemap[((1, 1), _ENDIAN + "f4")] = ("F", "F") -def open(fp, mode="r"): +def _compression_bomb_check(im, maximum_pixels): + if maximum_pixels is None: + return + + pixels = im.size[0] * im.size[1] + print("Pixels:", pixels) # FIXME: temporary + + if im.size[0] * im.size[1] > maximum_pixels: + raise ImageIsTooBigError("Image size exceeds limit") + + +def open(fp, mode="r", maximum_pixels=ARBITARY_LARGE_LIMIT): """ Opens and identifies the given image file. @@ -2114,6 +2131,7 @@ def open(fp, mode="r"): must implement :py:meth:`~file.read`, :py:meth:`~file.seek`, and :py:meth:`~file.tell` methods, and be opened in binary mode. :param mode: The mode. If given, this argument must be "r". + :param maximum_pixels: TODO. :returns: An :py:class:`~PIL.Image.Image` object. :exception IOError: If the file cannot be found, or the image cannot be opened and identified. @@ -2137,7 +2155,10 @@ def open(fp, mode="r"): factory, accept = OPEN[i] if not accept or accept(prefix): fp.seek(0) - return factory(fp, filename) + # return factory(fp, filename) + im = factory(fp, filename) + _compression_bomb_check(im, maximum_pixels) + return im except (SyntaxError, IndexError, TypeError): #import traceback #traceback.print_exc() @@ -2150,7 +2171,10 @@ def open(fp, mode="r"): factory, accept = OPEN[i] if not accept or accept(prefix): fp.seek(0) - return factory(fp, filename) + # return factory(fp, filename) + im = factory(fp, filename) + _compression_bomb_check(im, maximum_pixels) + return im except (SyntaxError, IndexError, TypeError): #import traceback #traceback.print_exc() From b7e5c276964898f00e21329b032fdb7147b41cc4 Mon Sep 17 00:00:00 2001 From: hugovk Date: Sat, 24 May 2014 17:52:48 +0300 Subject: [PATCH 02/11] Remove temporary print --- PIL/Image.py | 1 - 1 file changed, 1 deletion(-) diff --git a/PIL/Image.py b/PIL/Image.py index e33454d90..fac38640e 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2111,7 +2111,6 @@ def _compression_bomb_check(im, maximum_pixels): return pixels = im.size[0] * im.size[1] - print("Pixels:", pixels) # FIXME: temporary if im.size[0] * im.size[1] > maximum_pixels: raise ImageIsTooBigError("Image size exceeds limit") From 35f1f4d8fa9b290cfed877faef35336614a9c0eb Mon Sep 17 00:00:00 2001 From: hugovk Date: Mon, 26 May 2014 16:25:15 +0300 Subject: [PATCH 03/11] Change exception into a warning --- PIL/Image.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/PIL/Image.py b/PIL/Image.py index fac38640e..6450458bc 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -36,10 +36,7 @@ class _imaging_not_installed: raise ImportError("The _imaging C module is not installed") -class ImageIsTooBigError(Exception): - pass - -ARBITARY_LARGE_LIMIT = 6000 * 6000 - 1 # FIXME: Pick sensible limit +MAX_IMAGE_PIXELS = 6000 * 6000 - 1 # FIXME: Pick sensible limit try: # give Tk a chance to set up the environment, in case we're @@ -2106,17 +2103,21 @@ _fromarray_typemap[((1, 1), _ENDIAN + "i4")] = ("I", "I") _fromarray_typemap[((1, 1), _ENDIAN + "f4")] = ("F", "F") -def _compression_bomb_check(im, maximum_pixels): - if maximum_pixels is None: +def _compression_bomb_check(size): + if MAX_IMAGE_PIXELS is None: return - pixels = im.size[0] * im.size[1] + pixels = size[0] * size[1] - if im.size[0] * im.size[1] > maximum_pixels: - raise ImageIsTooBigError("Image size exceeds limit") + if pixels > MAX_IMAGE_PIXELS: + warnings.warn( + "Image size (%d pixels) exceeds limit of %d pixels, " + "could be decompression bomb DOS attack." % + (pixels, MAX_IMAGE_PIXELS), + RuntimeWarning) -def open(fp, mode="r", maximum_pixels=ARBITARY_LARGE_LIMIT): +def open(fp, mode="r"): """ Opens and identifies the given image file. @@ -2156,7 +2157,7 @@ def open(fp, mode="r", maximum_pixels=ARBITARY_LARGE_LIMIT): fp.seek(0) # return factory(fp, filename) im = factory(fp, filename) - _compression_bomb_check(im, maximum_pixels) + _compression_bomb_check(im.size) return im except (SyntaxError, IndexError, TypeError): #import traceback @@ -2172,7 +2173,7 @@ def open(fp, mode="r", maximum_pixels=ARBITARY_LARGE_LIMIT): fp.seek(0) # return factory(fp, filename) im = factory(fp, filename) - _compression_bomb_check(im, maximum_pixels) + _compression_bomb_check(im.size) return im except (SyntaxError, IndexError, TypeError): #import traceback From 29388f839582f35a541a7506c9818e68cf2bfb26 Mon Sep 17 00:00:00 2001 From: hugovk Date: Mon, 26 May 2014 16:26:42 +0300 Subject: [PATCH 04/11] Remove redundant comment [CI skip] --- PIL/Image.py | 1 - 1 file changed, 1 deletion(-) diff --git a/PIL/Image.py b/PIL/Image.py index 6450458bc..7014d36f3 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2131,7 +2131,6 @@ def open(fp, mode="r"): must implement :py:meth:`~file.read`, :py:meth:`~file.seek`, and :py:meth:`~file.tell` methods, and be opened in binary mode. :param mode: The mode. If given, this argument must be "r". - :param maximum_pixels: TODO. :returns: An :py:class:`~PIL.Image.Image` object. :exception IOError: If the file cannot be found, or the image cannot be opened and identified. From a0d8e5cb337de3dc921d54a90e0eb4b489da333e Mon Sep 17 00:00:00 2001 From: hugovk Date: Tue, 27 May 2014 12:10:10 +0300 Subject: [PATCH 05/11] Set limit to to around a quarter gigabyte for a 24 bit (3 bpp) image --- PIL/Image.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/PIL/Image.py b/PIL/Image.py index 7014d36f3..6dd9b2310 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -36,7 +36,8 @@ class _imaging_not_installed: raise ImportError("The _imaging C module is not installed") -MAX_IMAGE_PIXELS = 6000 * 6000 - 1 # FIXME: Pick sensible limit +# Limit to around a quarter gigabyte for a 24 bit (3 bpp) image +MAX_IMAGE_PIXELS = int(1024 * 1024 * 1024 / 4 / 3) try: # give Tk a chance to set up the environment, in case we're @@ -2157,6 +2158,7 @@ def open(fp, mode="r"): # return factory(fp, filename) im = factory(fp, filename) _compression_bomb_check(im.size) + print(im) return im except (SyntaxError, IndexError, TypeError): #import traceback From b853696ad54169c5242e635ec08504233cd5e946 Mon Sep 17 00:00:00 2001 From: hugovk Date: Tue, 27 May 2014 12:18:56 +0300 Subject: [PATCH 06/11] Remove stray debug print --- PIL/Image.py | 1 - 1 file changed, 1 deletion(-) diff --git a/PIL/Image.py b/PIL/Image.py index 6dd9b2310..f8977944b 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2158,7 +2158,6 @@ def open(fp, mode="r"): # return factory(fp, filename) im = factory(fp, filename) _compression_bomb_check(im.size) - print(im) return im except (SyntaxError, IndexError, TypeError): #import traceback From fd05e9c7560b0feb44e4a6cfd6d44ad28b8a1019 Mon Sep 17 00:00:00 2001 From: hugovk Date: Tue, 27 May 2014 12:40:52 +0300 Subject: [PATCH 07/11] Test decompression bomb warnings --- PIL/Image.py | 6 +++--- Tests/test_decompression_bomb.py | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 Tests/test_decompression_bomb.py diff --git a/PIL/Image.py b/PIL/Image.py index f8977944b..5ffa5b64b 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2104,7 +2104,7 @@ _fromarray_typemap[((1, 1), _ENDIAN + "i4")] = ("I", "I") _fromarray_typemap[((1, 1), _ENDIAN + "f4")] = ("F", "F") -def _compression_bomb_check(size): +def _decompression_bomb_check(size): if MAX_IMAGE_PIXELS is None: return @@ -2157,7 +2157,7 @@ def open(fp, mode="r"): fp.seek(0) # return factory(fp, filename) im = factory(fp, filename) - _compression_bomb_check(im.size) + _decompression_bomb_check(im.size) return im except (SyntaxError, IndexError, TypeError): #import traceback @@ -2173,7 +2173,7 @@ def open(fp, mode="r"): fp.seek(0) # return factory(fp, filename) im = factory(fp, filename) - _compression_bomb_check(im.size) + _decompression_bomb_check(im.size) return im except (SyntaxError, IndexError, TypeError): #import traceback diff --git a/Tests/test_decompression_bomb.py b/Tests/test_decompression_bomb.py new file mode 100644 index 000000000..1b3873c1b --- /dev/null +++ b/Tests/test_decompression_bomb.py @@ -0,0 +1,37 @@ +from tester import * + +from PIL import Image + +test_file = "Images/lena.ppm" + + +def test_no_warning_small_file(): + # Implicit assert: no warning. + # A warning would cause a failure. + Image.open(test_file) + + +def test_no_warning_no_limit(): + # Arrange + # Turn limit off + Image.MAX_IMAGE_PIXELS = None + assert_equal(Image.MAX_IMAGE_PIXELS, None) + + # Act / Assert + # Implicit assert: no warning. + # A warning would cause a failure. + Image.open(test_file) + + +def test_warning(): + # Arrange + # Set limit to a low, easily testable value + Image.MAX_IMAGE_PIXELS = 10 + assert_equal(Image.MAX_IMAGE_PIXELS, 10) + + # Act / Assert + assert_warning( + RuntimeWarning, + lambda: Image.open(test_file)) + +# End of file From d7ed249b29e4fab1c5aea41632cd5490cbf9a290 Mon Sep 17 00:00:00 2001 From: hugovk Date: Tue, 27 May 2014 14:39:33 +0300 Subject: [PATCH 08/11] Remove redundant commented code [CI skip] --- PIL/Image.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/PIL/Image.py b/PIL/Image.py index 5ffa5b64b..d1d5c510e 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2155,7 +2155,6 @@ def open(fp, mode="r"): factory, accept = OPEN[i] if not accept or accept(prefix): fp.seek(0) - # return factory(fp, filename) im = factory(fp, filename) _decompression_bomb_check(im.size) return im @@ -2171,7 +2170,6 @@ def open(fp, mode="r"): factory, accept = OPEN[i] if not accept or accept(prefix): fp.seek(0) - # return factory(fp, filename) im = factory(fp, filename) _decompression_bomb_check(im.size) return im From 46abe78b775e445974cfb9603640581b9fd9852c Mon Sep 17 00:00:00 2001 From: hugovk Date: Mon, 23 Jun 2014 10:53:08 +0300 Subject: [PATCH 09/11] Use a custom subclass of RuntimeWarning for DecompressionBombWarning --- PIL/Image.py | 5 ++++- Tests/test_decompression_bomb.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/PIL/Image.py b/PIL/Image.py index ce78bfa1b..b0b90a20b 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -31,6 +31,9 @@ from PIL import VERSION, PILLOW_VERSION, _plugins import warnings +class DecompressionBombWarning(RuntimeWarning): + pass + class _imaging_not_installed: # module placeholder def __getattr__(self, id): @@ -2187,7 +2190,7 @@ def _decompression_bomb_check(size): "Image size (%d pixels) exceeds limit of %d pixels, " "could be decompression bomb DOS attack." % (pixels, MAX_IMAGE_PIXELS), - RuntimeWarning) + DecompressionBombWarning) def open(fp, mode="r"): diff --git a/Tests/test_decompression_bomb.py b/Tests/test_decompression_bomb.py index 1b3873c1b..53c8bb5e6 100644 --- a/Tests/test_decompression_bomb.py +++ b/Tests/test_decompression_bomb.py @@ -31,7 +31,7 @@ def test_warning(): # Act / Assert assert_warning( - RuntimeWarning, + DecompressionBombWarning, lambda: Image.open(test_file)) # End of file From 7b3e242f09b128b5cf181cde69d4267ebf8761cb Mon Sep 17 00:00:00 2001 From: hugovk Date: Mon, 23 Jun 2014 11:12:41 +0300 Subject: [PATCH 10/11] Convert test_decompression_bomb.py to use unittest module --- Tests/test_decompression_bomb.py | 49 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/Tests/test_decompression_bomb.py b/Tests/test_decompression_bomb.py index 53c8bb5e6..1f85b1b9a 100644 --- a/Tests/test_decompression_bomb.py +++ b/Tests/test_decompression_bomb.py @@ -1,37 +1,40 @@ -from tester import * +from helper import unittest, PillowTestCase, tearDownModule from PIL import Image test_file = "Images/lena.ppm" -def test_no_warning_small_file(): - # Implicit assert: no warning. - # A warning would cause a failure. - Image.open(test_file) +class TestDecompressionBomb(PillowTestCase): + def test_no_warning_small_file(self): + # Implicit assert: no warning. + # A warning would cause a failure. + Image.open(test_file) -def test_no_warning_no_limit(): - # Arrange - # Turn limit off - Image.MAX_IMAGE_PIXELS = None - assert_equal(Image.MAX_IMAGE_PIXELS, None) + def test_no_warning_no_limit(self): + # Arrange + # Turn limit off + Image.MAX_IMAGE_PIXELS = None + self.assertEqual(Image.MAX_IMAGE_PIXELS, None) - # Act / Assert - # Implicit assert: no warning. - # A warning would cause a failure. - Image.open(test_file) + # Act / Assert + # Implicit assert: no warning. + # A warning would cause a failure. + Image.open(test_file) + def test_warning(self): + # Arrange + # Set limit to a low, easily testable value + Image.MAX_IMAGE_PIXELS = 10 + self.assertEqual(Image.MAX_IMAGE_PIXELS, 10) -def test_warning(): - # Arrange - # Set limit to a low, easily testable value - Image.MAX_IMAGE_PIXELS = 10 - assert_equal(Image.MAX_IMAGE_PIXELS, 10) + # Act / Assert + self.assert_warning( + Image.DecompressionBombWarning, + lambda: Image.open(test_file)) - # Act / Assert - assert_warning( - DecompressionBombWarning, - lambda: Image.open(test_file)) +if __name__ == '__main__': + unittest.main() # End of file From 282281f1e5778c18a7d2a9480224e3b113ec29d6 Mon Sep 17 00:00:00 2001 From: hugovk Date: Mon, 23 Jun 2014 11:22:25 +0300 Subject: [PATCH 11/11] Reset limit in tearDown() --- Tests/test_decompression_bomb.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tests/test_decompression_bomb.py b/Tests/test_decompression_bomb.py index 1f85b1b9a..b83f33f46 100644 --- a/Tests/test_decompression_bomb.py +++ b/Tests/test_decompression_bomb.py @@ -4,9 +4,14 @@ from PIL import Image test_file = "Images/lena.ppm" +ORIGINAL_LIMIT = Image.MAX_IMAGE_PIXELS + class TestDecompressionBomb(PillowTestCase): + def tearDown(self): + Image.MAX_IMAGE_PIXELS = ORIGINAL_LIMIT + def test_no_warning_small_file(self): # Implicit assert: no warning. # A warning would cause a failure.