GHSA-4558-3h8r-v8xp: Pip/Pillow vulnerability

Summary

On main, even with the guard in #9725, it's possible to have these decoders read 1 to 3 bytes past the end of the passed buffer with a suitable width/height.

Details

ImagingRawDecode and the P;2L / P;4L unpackers disagree about how many bytes a line of data occupies.

The raw decoder derives the line size from the bits per pixel

c state->bytes = (state->xsize state->bits + 7) / 8;

and uses it both for the bounds check and to advance to the next line. For P;2L and P;4L, bits is 2 and 4 respectively.

But these two rawmodes are not packed pixels but 2 or 4 bit planes, each padded to a whole number of bytes. unpackP2L computes s = (pixels + 7) / 8 and reads plane k at offset k s, and unpackP4L does the same across four planes. Their real footprint is therefore planes ((xsize + 7) / 8).

For some widths, padding each plane separately makes this footprint larger than state->bytes. For example, at width 1, the decoder accounts for one byte, while the unpackers read two or four bytes. The decoder accepts data that is too short for the last line and on the other hand advances to subsequent lines by less than a line, so every line after the first is also read from the wrong offset.

#9725's check is misshapen for this:

It can reject data of the right length: 8x1 P;2L image is 2 bytes but is not accepted. It still accepts buffers shorter than the unpackers require for a given image shape. It measures the length with len(data), which is an element count rather than a byte count; e.g. memoryview(array("I", [0,0,0,0,0])) (20 bytes) is rejected while mv.tobytes() is accepted.

PoC ( ghsap2lp4l.py)

python from array import array from PIL import Image

1 byte past Image.frombytes("P", (1, 4), array("B", [0] 4), "raw", "P;2L")

3 bytes past Image.frombytes("P", (1, 8), array("B", [0] 8), "raw", "P;4L") is the almost-simplest reproducer.

(array is needed to get ASan to reliably scream, but the overread is observable with e.g. bytes literals. Python allocates byteses with an extra trailing null byte, from an arena by default to boot, so ASan doesn't see the overread.)

On my Mac, I can get ASan to scream about this with

bash export asan=$(clang -print-file-name=libclangrt.asanosxdynamic.dylib) env CFLAGS="-fsanitize=address -fno-omit-frame-pointer -g -O1" LDFLAGS="-fsanitize=address" uv pip install -e . env DYLDINSERTLIBRARIES=$asan PYTHONMALLOC=malloc ASANOPTIONS=detectleaks=0 python3 ghsap2lp4l.py

like so:

<details>

python3(29144,0x202fea2c0) malloc: nano zone abandoned due to inability to reserve vm space. unpackP2L: in=0x602000037370 i=0 j=0 pixels=1 reads 0x602000037370 0x602000037371 unpackP2L: in=0x602000037371 i=0 j=0 pixels=1 reads 0x602000037371 0x602000037372 unpackP2L: in=0x602000037372 i=0 j=0 pixels=1 reads 0x602000037372 0x602000037373 unpackP2L: in=0x602000037373 i=0 j=0 pixels=1 reads 0x602000037373 0x602000037374 ================================================================= ==29144==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000037374 at pc 0x00010778c684 bp 0x00016d029f40 sp 0x00016d029f38 READ of size 1 at 0x602000037374 thread T0 #0 0x00010778c680 in unpackP2L+0x17c (imaging.cpython-314-darwin.so:arm64+0xb4680) #1 0x000107765360 in ImagingRawDecode+0x348 (imaging.cpython-314-darwin.so:arm64+0x8d360) #2 0x0001076f259c in decode+0x15c (imaging.cpython-314-darwin.so:arm64+0x1a59c) #3 0x0001030d921c in methodvectorcallVARARGS.llvm.8905850704545875072+0x84 (python3.14:arm64+0x10030521c) #4 0x000102e585b8 in TAILCALLCALL.llvm.12785051352303791014+0x13c (python3.14:arm64+0x1000845b8) #5 0x000102f1c17c in PyEvalVector+0x308 (python3.14:arm64+0x10014817c) #6 0x000102f31864 in PyEvalEvalCode+0x9c (python3.14:arm64+0x10015d864) #7 0x000102f3fa14 in runmod.llvm.1224844961852478530+0x120 (python3.14:arm64+0x10016ba14) #8 0x000103063eb0 in pyrunfile+0xa0 (python3.14:arm64+0x10028feb0) #9 0x000102dfe834 in PyRunSimpleFileObject+0xfc (python3.14:arm64+0x10002a834) #10 0x00010306cb60 in PyRunAnyFileObject+0x4c (python3.14:arm64+0x100298b60) #11 0x00010306ca54 in pymainrunfileobj+0xa0 (python3.14:arm64+0x100298a54) #12 0x000102ffb80c in pymainrunfile+0x44 (python3.14:arm64+0x10022780c) #13 0x000102ff7d0c in PyRunMain+0x46c (python3.14:arm64+0x100223d0c) #14 0x000102fb0820 in pymainmain.llvm.7308572997346072671+0x1cc (python3.14:arm64+0x1001dc820) #15 0x000102fb0644 in main+0x20 (python3.14:arm64+0x1001dc644) #16 0x000194c86b94 (<unknown module>)

0x602000037374 is located 0 bytes after 4-byte region [0x602000037370,0x602000037374) allocated by thread T0 here: #0 0x0001044a130c in malloc+0x78 (libclangrt.asanosxdynamic.dylib:arm64e+0x3d30c) #1 0x000102f22324 in newarrayobject+0x6c (python3.14:arm64+0x10014e324) #2 0x000102f21b4c in arraynew+0x320 (python3.14:arm64+0x10014db4c) #3 0x0001030fc390 in typecall+0x5c (python3.14:arm64+0x100328390) #4 0x000102e58ae4 in TAILCALLCALL.llvm.12785051352303791014+0x668 (python3.14:arm64+0x100084ae4) #5 0x000102f1c17c in PyEvalVector+0x308 (python3.14:arm64+0x10014817c) #6 0x000102f31864 in PyEvalEvalCode+0x9c (python3.14:arm64+0x10015d864) #7 0x000102f3fa14 in runmod.llvm.1224844961852478530+0x120 (python3.14:arm64+0x10016ba14) #8 0x000103063eb0 in pyrunfile+0xa0 (python3.14:arm64+0x10028feb0) #9 0x000102dfe834 in PyRunSimpleFileObject+0xfc (python3.14:arm64+0x10002a834) #10 0x00010306cb60 in PyRunAnyFileObject+0x4c (python3.14:arm64+0x100298b60) #11 0x00010306ca54 in pymainrunfileobj+0xa0 (python3.14:arm64+0x100298a54) #12 0x000102ffb80c in pymainrunfile+0x44 (python3.14:arm64+0x10022780c) #13 0x000102ff7d0c in PyRunMain+0x46c (python3.14:arm64+0x100223d0c) #14 0x000102fb0820 in pymainmain.llvm.7308572997346072671+0x1cc (python3.14:arm64+0x1001dc820) #15 0x000102fb0644 in main+0x20 (python3.14:arm64+0x1001dc644) #16 0x000194c86b94 (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow (imaging.cpython-314-darwin.so:arm64+0xb4680) in unpackP2L+0x17c Shadow bytes around the buggy address: 0x602000037080: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x602000037100: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x602000037180: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x602000037200: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x602000037280: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa =>0x602000037300: fa fa 00 fa fa fa 00 00 fa fa fd fa fa fa[04]fa 0x602000037380: fa fa 04 fa fa fa fd fd fa fa fd fd fa fa fd fd 0x602000037400: fa fa fd fd fa fa fd fd fa fa fd fa fa fa 00 fa 0x602000037480: fa fa 01 fa fa fa fa fa fa fa fa fa fa fa fa fa 0x602000037500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x602000037580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb </details>

(To get it to scream differently about unpackP4L, comment out the P;2L case.)

The above output is with unpackP2L and unpackP4L off 5eee92031a1986dfdaac7c78cdf0b764c0697118 lazily instrumented to print out what it's reading:

<details>

patch diff --git a/src/libImaging/Unpack.c b/src/libImaging/Unpack.c index a51568209..4f867737a 100644 --- a/src/libImaging/Unpack.c +++ b/src/libImaging/Unpack.c @@ -548,6 +548,8 @@ unpackP4(UINT8 out, const UINT8 in, int pixels) { } } +#include <stdio.h> + static void unpackP2L(UINT8 out, const UINT8 in, int pixels) { int i, j, m, s; @@ -555,6 +557,8 @@ unpackP2L(UINT8 out, const UINT8 in, int pixels) { m = 128; s = (pixels + 7) / 8; for (i = j = 0; i < pixels; i++) { + fprintf(stderr, "unpackP2L: in=%p i=%d j=%d pixels=%d reads %p %p\n", + in, i, j, pixels, in + j, in + j + s); out[i] = ((in[j] & m) ? 1 : 0) + ((in[j + s] & m) ? 2 : 0); if ((m >>= 1) == 0) { m = 128; @@ -570,6 +574,8 @@ unpackP4L(UINT8 out, const UINT8 in, int pixels) { m = 128; s = (pixels + 7) / 8; for (i = j = 0; i < pixels; i++) { + fprintf(stderr, "unpackP4L: in=%p i=%d j=%d pixels=%d reads %p %p %p %p\n", + in, i, j, pixels, in + j, in + j + s, in + j + 2 s, in + j + 3 s); out[i] = ((in[j] & m) ? 1 : 0) + ((in[j + s] & m) ? 2 : 0) + ((in[j + 2 s] & m) ? 4 : 0) + ((in[j + 3 s] & m) ? 8 : 0); if ((m >>= 1) == 0) { </details>

Impact

Only a very specific use case is affected: Image.frombytes() or Image.frombuffer() calls of the shape above. The PCX decoder, which is the only Pillow-internal user of the P;2L and P;4L unpackers, already re-packs PCX rows into a format where overreads aren't an issue.

The impact is that up to 1 byte (P;2L) or 3 bytes (P;4L) of heap memory following the caller's buffer are read and mixed into the pixel values of the last decoded row(s), one bit per affected plane per pixel. A crash is also possible if the read reaches unmapped memory.

As far as I can tell, all published Pillow versions are affected.

Notes

These findings were momentarily public (though in no detail) in https://github.com/python-pillow/Pillow/pull/9723#issuecomment-5566006996; Andrew asked to retract the comment and post this instead.