CVE-2023-4863 POC image causes Pillow 10.0.1 to throw OSError

[alienth]

What did you do?

Generated a CVE-2023-4863 POC image, and attempted to open it with Pillow 10.0.1:

curl -O https://raw.githubusercontent.com/mistymntncop/CVE-2023-4863/main/craft.c
gcc -o craft ./craft.c
./craft bad.webp

>>> from PIL import Image
>>> Image.open('/tmp/bad.webp')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/alienth/venv/lib/python3.10/site-packages/PIL/Image.py", line 3264, in open
    im = _open_core(
  File "/home/alienth/venv/lib/python3.10/site-packages/PIL/Image.py", line 3245, in _open_core
    im = factory(fp, filename)
  File "/home/alienth/venv/lib/python3.10/site-packages/PIL/ImageFile.py", line 117, in __init__
    self._open()
  File "/home/alienth/venv/lib/python3.10/site-packages/PIL/WebPImagePlugin.py", line 62, in _open
    self._decoder = _webp.WebPAnimDecoder(self.fp.read())
OSError: could not create decoder object
>>> Image.__version__
'10.0.1'

What did you expect to happen?

Some more accurate exception to be thrown, such as PIL.UnidentifiedImageError.

What actually happened?

OSError thrown when trying to create Animation decoder.

What are your OS, Python and Pillow versions?

• OS: Ubuntu Jammy
• Python: 3.10.12
• Pillow: 10.0.1

from PIL import Image
Image.open('/tmp/bad.webp')

[radarhere]

Hi. Just to be clear, what you're talking about is not a security concern, and you're not suggesting that UnidentifiedImageError is what should happen here.

You're interested in a Pillow-specific error type being raised, rather than a generic OSError? This is very similar to, if not a duplicate, of #1643.

[alienth]

Yep, not a security concern, just found it odd that this is the exception which would be thrown. Seemed like an odd exception to throw, under the circumstances. Also not in the list of documented exceptions for the open function.

There is some minor risk of a DoS here, where someone uploads the POC or exploit image to cause an exception that an app using Pillow isn't expecting to handle.

[alienth]

Minor curious note: Pillow 10.0.0 throws the same exception with the POC image, despite using the vulnerable libwebp.

[wiredfool]

There are a couple of potential reasons for that -- which could be narrowed down by using valgrind.

1. Pillow wasn't vulnerable to that particular file, or through some quirk, was not vulnerable
2. The test file throws that error anyway, after overflowing the heap in a manner that wasn't detected.

[zsims]

Minor curious note: Pillow 10.0.0 throws the same exception with the POC image, despite using the vulnerable libwebp.

The OOB write may not cause a crash depending on your allocator (glibc vs whatever is in macOS) and other environmental factors. To verify with glibc you can use MALLOC_CHECK_ without having to rebuild Python:

$ pip install Pillow==10.0.0
$ LD_PRELOAD="/usr/lib64/libc_malloc_debug.so.0" MALLOC_CHECK_=2 python3 -c 'import PIL.Image; PIL.Image.open("bad.webp").convert("RGB")'
free(): invalid next size (normal)
Aborted (core dumped)

And/or you may need to modify the PoC to write further OOB to reliably trigger.

[radarhere]

Pillow values backwards compatibility, so replacing the OSError with a completely different error would not be ideal. If you are suggesting a custom error that subclasses OSError, that sounds like a solution, but one that should be considered at the same time as #1643.

Regarding the Image.open docstring, it lists the exceptions that are thrown by the method itself. The OSError comes from WebPImagePlugin.

Listing info on every exception raised by every plugin when opening a file would be rather long. Given that many Pillow methods call load() internally, which calls further plugin code, I don't think listing every exception for every method in our documentation is a standard that we can meet without a lot of repetition.

[alienth]

Listing info on every exception raised by every plugin when opening a file would be rather long. Given that many Pillow methods call load() internally, which calls further plugin code, I don't think listing every exception for every method in our documentation is a standard that we can meet without a lot of repetition.

That's fair.

If you are suggesting a custom error that subclasses OSError, that sounds like a solution, but one that should be considered at the same time as #1643.

Yeah, I think a custom error would allay my concerns. If that's the goal of #1643 and you feel it covers this case, then I have no objection to marking this issue as a duplicate of that one and closing.

[radarhere]

Thanks. I've made a comment there.