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      
       
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    

        [,

    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    

    

                  
                  
                  
                  
                  
              []
                  
                  
                  
                  
                  

             
         
         
         
        
         
       
        
     
            
         
          
        
              
      
             
       
      

</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/