Skip to content

Commit

Permalink
Fix DOS in PSDImagePlugin -- CVE-2021-28675
Browse files Browse the repository at this point in the history
* PSDImagePlugin did not sanity check the number of input layers and
  vs the size of the data block, this could lead to a DOS on
  Image.open prior to Image.load.
* This issue dates to the PIL fork
  • Loading branch information
wiredfool authored and hugovk committed Apr 1, 2021
1 parent ba65f0b commit 22e9bee
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 17 deletions.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions Tests/test_decompression_bomb.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def test_exception(self):
with Image.open(TEST_FILE):
pass

@pytest.mark.xfail(reason="different exception")
def test_exception_ico(self):
with pytest.raises(Image.DecompressionBombError):
with Image.open("Tests/images/decompression_bomb.ico"):
Expand Down
2 changes: 1 addition & 1 deletion Tests/test_file_apng.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ def open_frames_zero_default():
exception = e
assert exception is None

with pytest.raises(SyntaxError):
with pytest.raises(OSError):
with Image.open("Tests/images/apng/syntax_num_frames_high.png") as im:
im.seek(im.n_frames - 1)
im.load()
Expand Down
1 change: 1 addition & 0 deletions Tests/test_file_blp.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from PIL import Image
import pytest

from .helper import assert_image_equal_tofile

Expand Down
15 changes: 15 additions & 0 deletions Tests/test_file_psd.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,18 @@ def test_combined_larger_than_size():
with pytest.raises(OSError):
with Image.open("Tests/images/combined_larger_than_size.psd"):
pass

@pytest.mark.parametrize(
"test_file,raises",
[
("Tests/images/timeout-1ee28a249896e05b83840ae8140622de8e648ba9.psd", Image.UnidentifiedImageError),
("Tests/images/timeout-598843abc37fc080ec36a2699ebbd44f795d3a6f.psd", Image.UnidentifiedImageError),
("Tests/images/timeout-c8efc3fded6426986ba867a399791bae544f59bc.psd", OSError),
("Tests/images/timeout-dedc7a4ebd856d79b4359bbcc79e8ef231ce38f6.psd", OSError),
],
)
def test_crashes(test_file, raises):
with open(test_file, "rb") as f:
with pytest.raises(raises):
with Image.open(f):
pass
7 changes: 4 additions & 3 deletions Tests/test_file_tiff.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,9 +625,10 @@ def test_close_on_load_nonexclusive(self, tmp_path):
)
def test_string_dimension(self):
# Assert that an error is raised if one of the dimensions is a string
with pytest.raises(ValueError):
with Image.open("Tests/images/string_dimension.tiff"):
pass
with pytest.raises(OSError):
with Image.open("Tests/images/string_dimension.tiff") as im:
im.load()



@pytest.mark.skipif(not is_win32(), reason="Windows only")
Expand Down
14 changes: 12 additions & 2 deletions src/PIL/ImageFile.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,22 +545,32 @@ def _safe_read(fp, size):
:param fp: File handle. Must implement a <b>read</b> method.
:param size: Number of bytes to read.
:returns: A string containing up to <i>size</i> bytes of data.
:returns: A string containing <i>size</i> bytes of data.
Raises an OSError if the file is truncated and the read can not be completed
"""
if size <= 0:
return b""
if size <= SAFEBLOCK:
return fp.read(size)
data = fp.read(size)
if len(data) < size:
raise OSError("Truncated File Read")
return data
data = []
while size > 0:
block = fp.read(min(size, SAFEBLOCK))
if not block:
break
data.append(block)
size -= len(block)
if sum(len(d) for d in data) < size:
raise OSError("Truncated File Read")
return b"".join(data)




class PyCodecState:
def __init__(self):
self.xsize = 0
Expand Down
32 changes: 21 additions & 11 deletions src/PIL/PsdImagePlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ def _open(self):
end = self.fp.tell() + size
size = i32(read(4))
if size:
self.layers = _layerinfo(self.fp)
_layer_data = io.BytesIO(ImageFile._safe_read(self.fp, size))
self.layers = _layerinfo(_layer_data, size)
self.fp.seek(end)
self.n_frames = len(self.layers)
self.is_animated = self.n_frames > 1
Expand Down Expand Up @@ -170,12 +171,20 @@ def _close__fp(self):
finally:
self.__fp = None


def _layerinfo(file):
def _layerinfo(fp, ct_bytes):
# read layerinfo block
layers = []
read = file.read
for i in range(abs(i16(read(2)))):

def read(size):
return ImageFile._safe_read(fp, size)

ct = i16(read(2))

# sanity check
if ct_bytes < (abs(ct) * 20):
raise SyntaxError("Layer block too short for number of layers requested")

for i in range(abs(ct)):

# bounding box
y0 = i32(read(4))
Expand All @@ -186,7 +195,8 @@ def _layerinfo(file):
# image info
info = []
mode = []
types = list(range(i16(read(2))))
ct_types = i16(read(2))
types = list(range(ct_types))
if len(types) > 4:
continue

Expand Down Expand Up @@ -219,16 +229,16 @@ def _layerinfo(file):
size = i32(read(4)) # length of the extra data field
combined = 0
if size:
data_end = file.tell() + size
data_end = fp.tell() + size

length = i32(read(4))
if length:
file.seek(length - 16, io.SEEK_CUR)
fp.seek(length - 16, io.SEEK_CUR)
combined += length + 4

length = i32(read(4))
if length:
file.seek(length, io.SEEK_CUR)
fp.seek(length, io.SEEK_CUR)
combined += length + 4

length = i8(read(1))
Expand All @@ -238,15 +248,15 @@ def _layerinfo(file):
name = read(length).decode("latin-1", "replace")
combined += length + 1

file.seek(data_end)
fp.seek(data_end)
layers.append((name, mode, (x0, y0, x1, y1)))

# get tiles
i = 0
for name, mode, bbox in layers:
tile = []
for m in mode:
t = _maketile(file, m, bbox, 1)
t = _maketile(fp, m, bbox, 1)
if t:
tile.extend(t)
layers[i] = name, mode, bbox, tile
Expand Down

0 comments on commit 22e9bee

Please # to comment.