From da633723d4f4715c58ba7290ddb98695f6861099 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Sun, 23 Aug 2026 15:47:37 +0500 Subject: [PATCH 1/3] Fix OOB read in getlist() and putdata() when __len__ overstates length --- Tests/test_image_point.py | 20 ++++++++++++++++++++ Tests/test_image_putdata.py | 18 ++++++++++++++++++ src/_imaging.c | 37 +++++++++++++++++++------------------ 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/Tests/test_image_point.py b/Tests/test_image_point.py index a5d5a15dbab..24640c9a349 100644 --- a/Tests/test_image_point.py +++ b/Tests/test_image_point.py @@ -2,6 +2,8 @@ import pytest +from PIL import Image + from .helper import assert_image_equal, hopper @@ -62,3 +64,21 @@ def test_f_mode() -> None: im = hopper("F") with pytest.raises(ValueError): im.point([]) + + +def test_overstated_length() -> None: + # shouldn't segfault + # see https://github.com/python-pillow/Pillow/issues/9892 + + class OverstatedLengthSequence: + def __len__(self) -> int: + return 256 + + def __getitem__(self, index: int) -> float: + if index >= 8: + raise IndexError + return float(index) + + im = Image.new("L", (4, 4)) + with pytest.raises(ValueError): + im.point(OverstatedLengthSequence(), "F") diff --git a/Tests/test_image_putdata.py b/Tests/test_image_putdata.py index 1e727316d1d..ab54c1a5d1c 100644 --- a/Tests/test_image_putdata.py +++ b/Tests/test_image_putdata.py @@ -99,6 +99,24 @@ def test_array_F() -> None: assert len(im.get_flattened_data()) == len(arr) +def test_overstated_length() -> None: + # shouldn't segfault + # see https://github.com/python-pillow/Pillow/issues/9892 + + class OverstatedLengthSequence: + def __len__(self) -> int: + return 16 + + def __getitem__(self, index: int) -> float: + if index >= 2: + raise IndexError + return float(index + 1) + + im = Image.new("L", (4, 4)) + im.putdata(OverstatedLengthSequence()) + assert im.get_flattened_data()[:2] == (1, 2) + + def test_not_flattened() -> None: im = Image.new("L", (1, 1)) with pytest.raises(TypeError): diff --git a/src/_imaging.c b/src/_imaging.c index 9bdb6328782..f4004434767 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -442,9 +442,15 @@ getlist(PyObject *arg, Py_ssize_t *length, const char *wrong_length, int type) { return NULL; } - n = PySequence_Size(arg); + seq = PySequence_Fast(arg, must_be_sequence); + if (!seq) { + return NULL; + } + + n = PySequence_Fast_GET_SIZE(seq); if (length && wrong_length && n != *length) { PyErr_SetString(PyExc_ValueError, wrong_length); + Py_DECREF(seq); return NULL; } @@ -452,15 +458,10 @@ getlist(PyObject *arg, Py_ssize_t *length, const char *wrong_length, int type) { calloc checks for overflow */ list = calloc(n, type & 0xff); if (!list) { + Py_DECREF(seq); return ImagingError_MemoryError(); } - seq = PySequence_Fast(arg, must_be_sequence); - if (!seq) { - free(list); - return NULL; - } - for (i = 0; i < n; i++) { op = PySequence_Fast_GET_ITEM(seq, i); // DRY, branch prediction is going to work _really_ well @@ -1625,8 +1626,18 @@ _putdata(ImagingObject *self, PyObject *args) { image = self->image; - n = PyObject_Length(data); + if (image->image8 && PyBytes_Check(data)) { + n = PyBytes_GET_SIZE(data); + } else { + seq = PySequence_Fast(data, must_be_sequence); + if (!seq) { + PyErr_SetString(PyExc_TypeError, must_be_sequence); + return NULL; + } + n = PySequence_Fast_GET_SIZE(seq); + } if (n > (Py_ssize_t)image->xsize * (Py_ssize_t)image->ysize) { + Py_XDECREF(seq); PyErr_SetString(PyExc_TypeError, "too many data entries"); return NULL; } @@ -1667,11 +1678,6 @@ _putdata(ImagingObject *self, PyObject *args) { } } } else { - seq = PySequence_Fast(data, must_be_sequence); - if (!seq) { - PyErr_SetString(PyExc_TypeError, must_be_sequence); - return NULL; - } double value; int bigendian = 0; if (image->type == IMAGING_TYPE_SPECIAL) { @@ -1705,11 +1711,6 @@ _putdata(ImagingObject *self, PyObject *args) { } } else { /* 32-bit images */ - seq = PySequence_Fast(data, must_be_sequence); - if (!seq) { - PyErr_SetString(PyExc_TypeError, must_be_sequence); - return NULL; - } switch (image->type) { case IMAGING_TYPE_INT32: for (i = x = y = 0; i < n; i++) { From bab52a85305d556db4be02e16205646a76a41e13 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Sun, 23 Aug 2026 16:00:58 +0500 Subject: [PATCH 2/3] Reject mismatched-length sequences before materializing them --- Tests/test_image_point.py | 2 +- Tests/test_image_putdata.py | 2 +- src/_imaging.c | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Tests/test_image_point.py b/Tests/test_image_point.py index 24640c9a349..085104766ca 100644 --- a/Tests/test_image_point.py +++ b/Tests/test_image_point.py @@ -81,4 +81,4 @@ def __getitem__(self, index: int) -> float: im = Image.new("L", (4, 4)) with pytest.raises(ValueError): - im.point(OverstatedLengthSequence(), "F") + im.point(OverstatedLengthSequence(), "F") # type: ignore[arg-type] diff --git a/Tests/test_image_putdata.py b/Tests/test_image_putdata.py index ab54c1a5d1c..325336bcd9d 100644 --- a/Tests/test_image_putdata.py +++ b/Tests/test_image_putdata.py @@ -113,7 +113,7 @@ def __getitem__(self, index: int) -> float: return float(index + 1) im = Image.new("L", (4, 4)) - im.putdata(OverstatedLengthSequence()) + im.putdata(OverstatedLengthSequence()) # type: ignore[arg-type] assert im.get_flattened_data()[:2] == (1, 2) diff --git a/src/_imaging.c b/src/_imaging.c index f4004434767..8f0cb99d9d5 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -442,6 +442,16 @@ getlist(PyObject *arg, Py_ssize_t *length, const char *wrong_length, int type) { return NULL; } + if (length && wrong_length) { + Py_ssize_t reported = PySequence_Size(arg); + if (reported < 0) { + PyErr_Clear(); + } else if (reported != *length) { + PyErr_SetString(PyExc_ValueError, wrong_length); + return NULL; + } + } + seq = PySequence_Fast(arg, must_be_sequence); if (!seq) { return NULL; @@ -1629,6 +1639,14 @@ _putdata(ImagingObject *self, PyObject *args) { if (image->image8 && PyBytes_Check(data)) { n = PyBytes_GET_SIZE(data); } else { + Py_ssize_t reported = PySequence_Size(data); + if (reported < 0) { + PyErr_Clear(); + } else if (reported > (Py_ssize_t)image->xsize * (Py_ssize_t)image->ysize) { + PyErr_SetString(PyExc_TypeError, "too many data entries"); + return NULL; + } + seq = PySequence_Fast(data, must_be_sequence); if (!seq) { PyErr_SetString(PyExc_TypeError, must_be_sequence); From 89654d1d68c4264f3d873af8dd3417347de73006 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Mon, 24 Aug 2026 13:10:34 +0500 Subject: [PATCH 3/3] Add regression tests for getlist()/putdata() pre-check branches codecov/patch was failing at 64.71% because the self-review's cheap PySequence_Size() pre-check (added to reject honest-but-oversized sequences in O(1)) had no dedicated test: the "too many data entries" TypeError path in _putdata() and the "no __len__" fallback path in both getlist() and _putdata() (common for custom point-table-like objects) were completely untested anywhere in the suite. Add test_too_many_entries (putdata) and test_unsized_sequence (point, putdata) to close those gaps; mirrors the existing test_overstated_length convention. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01R51WpQNzUqKreMEWv9TZtd --- Tests/test_image_point.py | 15 +++++++++++++++ Tests/test_image_putdata.py | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/Tests/test_image_point.py b/Tests/test_image_point.py index 085104766ca..0d1c8dae4d3 100644 --- a/Tests/test_image_point.py +++ b/Tests/test_image_point.py @@ -82,3 +82,18 @@ def __getitem__(self, index: int) -> float: im = Image.new("L", (4, 4)) with pytest.raises(ValueError): im.point(OverstatedLengthSequence(), "F") # type: ignore[arg-type] + + +def test_unsized_sequence() -> None: + # a table that only implements __getitem__ (no __len__) is a common shape + # for custom point tables; PySequence_Size() fails for it, so the cheap + # pre-check must fall back to materializing instead of erroring out + class UnsizedSequence: + def __getitem__(self, index: int) -> float: + if index >= 256: + raise IndexError + return float(index) + + im = Image.new("L", (4, 4)) + out = im.point(UnsizedSequence(), "F") # type: ignore[arg-type] + assert_image_equal(out, im.point(list(range(256)), "F")) diff --git a/Tests/test_image_putdata.py b/Tests/test_image_putdata.py index 325336bcd9d..601b2eb7e2f 100644 --- a/Tests/test_image_putdata.py +++ b/Tests/test_image_putdata.py @@ -117,6 +117,29 @@ def __getitem__(self, index: int) -> float: assert im.get_flattened_data()[:2] == (1, 2) +def test_too_many_entries() -> None: + # an honest, correctly-reported sequence that is simply longer than the + # image still has to be rejected, before or after materialization + im = Image.new("L", (4, 4)) + with pytest.raises(TypeError): + im.putdata(list(range(17))) + + +def test_unsized_sequence() -> None: + # a sequence that only implements __getitem__ (no __len__) is common for + # custom point-table-like objects; PySequence_Size() fails for it, so the + # cheap pre-check must fall back to materializing instead of erroring out + class UnsizedSequence: + def __getitem__(self, index: int) -> int: + if index >= 4: + raise IndexError + return index + + im = Image.new("L", (2, 2)) + im.putdata(UnsizedSequence()) # type: ignore[arg-type] + assert im.get_flattened_data() == (0, 1, 2, 3) + + def test_not_flattened() -> None: im = Image.new("L", (1, 1)) with pytest.raises(TypeError):