Harden planar row-size checks in PcxDecode and ZipDecode

Tests/test_lib_pack.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import sys
+import zlib
 
 import pytest
 
@@ -858,3 +859,86 @@ def test_pXl_decoder_buffer_size(
         im = Image.new("P", (9, 1))
         with pytest.raises(ValueError, match="not enough image data"):
             im.frombytes(data, decoder_name, rawmode)
+
+    @pytest.mark.parametrize(
+        "size, rawmode, data, stride",
+        (
+            ((9, 1), "P;2L", b"\x01\x02\x03", 3),  # packed size 3, planar size 4
+            ((17, 1), "P;4L", bytes(range(1, 10)), 9),  # packed 9, planar 12
+        ),
+    )
+    def test_pcx_pXl_decoder_buffer_size(
+        self, size: tuple[int, int], rawmode: str, data: bytes, stride: int
+    ) -> None:
+        # The PCX decoder takes its buffer size from the stride argument, so it
+        # must check against the planar row size itself
+        im = Image.new("P", size)
+        with pytest.raises(ValueError, match="cannot decode image data"):
+            im.frombytes(data, "pcx", rawmode, stride)
+
+    def test_pcx_packed_decoder_buffer_size(self) -> None:
+        # Packed rows use the packed row size, not the stride argument
+        im = Image.new("P", (9, 1))
+        with pytest.raises(ValueError, match="cannot decode image data"):
+            im.frombytes(b"\x94\x60\x80", "pcx", "P;2", 1)
+
+    @pytest.mark.parametrize(
+        "size, rawmode, data, stride",
+        (
+            ((9, 1), "P;2L", b"\x01\x02\x03\x04", 4),
+            ((17, 1), "P;4L", bytes(range(1, 13)), 12),
+        ),
+    )
+    def test_pcx_pXl_decoder_buffer_size_ok(
+        self, size: tuple[int, int], rawmode: str, data: bytes, stride: int
+    ) -> None:
+        im = Image.new("P", size)
+        im.frombytes(data, "pcx", rawmode, stride)
+
+    @pytest.mark.parametrize(
+        "mode, rawmode, size, data, stride, expected",
+        (
+            # Packed 2/4-bit rows keep their packed layout when the row is
+            # padded: only the planar raw modes (P;2L/P;4L) store padded bit
+            # planes, so the row collapse must not de-interleave these rows
+            (
+                "P",
+                "P;2",
+                (9, 1),
+                b"\x94\x60\x80\x40\x00\x00",
+                6,
+                (2, 1, 1, 0, 1, 2, 0, 0, 2),
+            ),
+            (
+                "P",
+                "P;4",
+                (5, 1),
+                b"\x12\x34\x50\x00\x00\x00\x00\x00",
+                8,
+                (1, 2, 3, 4, 5),
+            ),
+        ),
+    )
+    def test_pcx_packed_low_bit_padded_row(
+        self,
+        mode: str,
+        rawmode: str,
+        size: tuple[int, int],
+        data: bytes,
+        stride: int,
+        expected: tuple[int, ...],
+    ) -> None:
+        # Packed P;2/P;4 rows are not bit planes; a padded stride must
+        # not change the decoded pixels (the rows are de-interleaved only for
+        # the planar raw modes)
+        im = Image.new(mode, size)
+        im.frombytes(data, "pcx", rawmode, stride)
+        assert tuple(im.get_flattened_data()) == expected
+
+    @pytest.mark.parametrize("rawmode", ("P;2L", "P;4L"))
+    def test_zip_pXl_interlaced_rejected(self, rawmode: str) -> None:
+        # The interlaced unpacker stages a single byte per pixel; planar raw
+        # modes would read past it
+        im = Image.new("P", (16, 16))
+        with pytest.raises(ValueError, match="unrecognized raw mode"):
+            im.frombytes(zlib.compress(b"\x00" * 32), "zip", rawmode, 1)

docs/releasenotes/13.1.0.rst

@@ -50,6 +50,15 @@ TODO
 Other changes
 =============
 
+PCX and ZIP decoder row sizes
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When using :py:meth:`~PIL.Image.Image.frombytes()` with the ``pcx`` decoder and a
+caller-supplied stride, packed 2- and 4-bit rows padded to a byte boundary were
+collapsed incorrectly, and the row-size arithmetic could overflow for very wide
+images. The ZIP decoder no longer accepts planar raw modes when interlacing is
+requested, since it cannot decode them correctly.
+
 Writing to read-only images
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

src/decode.c

@@ -839,6 +839,16 @@ PyImaging_ZipDecoderNew(PyObject *self, PyObject *args) {
         return NULL;
     }
 
+    // The interlaced path feeds the unpacker one pixel at a time through a
+    // single byte staging slot, which is incompatible with planar raw modes,
+    // so reject this combination now.
+    if (interlaced &&
+        (rawmode == IMAGING_RAWMODE_P_2L || rawmode == IMAGING_RAWMODE_P_4L)) {
+        Py_DECREF(decoder);
+        PyErr_SetString(PyExc_ValueError, "unrecognized raw mode");
+        return NULL;
+    }
+
     decoder->decode = ImagingZipDecode;
     decoder->cleanup = ImagingZipDecodeCleanup;
 

src/libImaging/PcxDecode.c

@@ -20,9 +20,20 @@ ImagingPcxDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t byt
     UINT8 n;
     UINT8 *ptr;
 
-    if ((state->xsize * state->bits + 7) / 8 > state->bytes) {
-        state->errcode = IMAGING_CODEC_OVERRUN;
-        return -1;
+    /* Check that the buffer holds one line of input. Planar raw modes
+       (P;2L/P;4L) store each line as state->planes padded bit planes,
+       not as packed pixels (#10186). The row size is computed in 64-bit
+       arithmetic so that a very wide image cannot overflow it. */
+    if (state->planes) {
+        if ((int64_t)state->planes * (((int64_t)state->xsize + 7) / 8) > state->bytes) {
+            state->errcode = IMAGING_CODEC_OVERRUN;
+            return -1;
+        }
+    } else {
+        if (((int64_t)state->bits * state->xsize + 7) / 8 > state->bytes) {
+            state->errcode = IMAGING_CODEC_OVERRUN;
+            return -1;
+        }
     }
 
     ptr = buf;
@@ -62,9 +73,9 @@ ImagingPcxDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t byt
         if (state->x >= state->bytes) {
             int bands;
             int xsize = 0;
-            if (state->bits == 2 || state->bits == 4) {
+            if (state->planes) {
                 xsize = (state->xsize + 7) / 8;
-                bands = state->bits;
+                bands = state->planes;
             } else {
                 xsize = state->xsize;
                 bands = im->bands;