Skip to content

Commit

Permalink
Fix for CVE CVE-2020-35655 - Read Overflow in PCX Decoding.
Browse files Browse the repository at this point in the history
* Don't trust the image to specify a buffer size
  • Loading branch information
wiredfool authored and radarhere committed Jan 2, 2021
1 parent 0c39689 commit 2f40926
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
Binary file added Tests/images/ossfuzz-4836216264589312.pcx
Binary file not shown.
27 changes: 15 additions & 12 deletions Tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,26 +775,29 @@ def test_pillow_version(self, test_module):
with pytest.warns(DeprecationWarning):
assert test_module.PILLOW_VERSION > "7.0.0"

def test_overrun(self):
"""For overrun completeness, test as:
valgrind pytest -qq Tests/test_image.py::TestImage::test_overrun | grep decode.c
"""
for file in [
@pytest.mark.parametrize("path", [
"fli_overrun.bin",
"sgi_overrun.bin",
"sgi_overrun_expandrow.bin",
"sgi_overrun_expandrow2.bin",
"pcx_overrun.bin",
"pcx_overrun2.bin",
"ossfuzz-4836216264589312.pcx",
"01r_00.pcx",
]:
with Image.open(os.path.join("Tests/images", file)) as im:
try:
im.load()
assert False
except OSError as e:
assert str(e) == "buffer overrun when reading image file"
])
def test_overrun(self, path):
"""For overrun completeness, test as:
valgrind pytest -qq Tests/test_image.py::TestImage::test_overrun | grep decode.c
"""
with Image.open(os.path.join("Tests/images", path)) as im:
try:
im.load()
assert False
except OSError as e:
assert (str(e) == "buffer overrun when reading image file" or
"image file is truncated" in str(e))

def test_fli_overrun2(self):
with Image.open("Tests/images/fli_overrun2.bin") as im:
try:
im.seek(1)
Expand Down
9 changes: 7 additions & 2 deletions src/PIL/PcxImagePlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ def _open(self):
version = s[1]
bits = s[3]
planes = s[65]
stride = i16(s, 66)
ignored_stride = i16(s, 66)
logger.debug(
"PCX version %s, bits %s, planes %s, stride %s",
version,
bits,
planes,
stride,
ignored_stride,
)

self.info["dpi"] = i16(s, 12), i16(s, 14)
Expand Down Expand Up @@ -110,6 +110,11 @@ def _open(self):
self.mode = mode
self._size = bbox[2] - bbox[0], bbox[3] - bbox[1]

# don't trust the passed in stride. Calculate for ourselves.
# CVE-2020-35655
stride = (self._size[0] * bits + 7) // 8
stride += stride % 2

bbox = (0, 0) + self.size
logger.debug("size: %sx%s", *self.size)

Expand Down

1 comment on commit 2f40926

@tcullum-rh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This commit is labeled as CVE-2020-35655 but per https://pillow.readthedocs.io/en/stable/releasenotes/8.1.0.html#security this should be CVE-2020-35653. CVE-2020-35655 Fix for SGI Decode buffer overrun which is also the case on MITRE CVE page.

Please # to comment.