From f25ce8a93384cb87c5cd5f071ebac37ed06e4335 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 25 Oct 2025 21:09:20 -0700 Subject: [PATCH 1/7] gh-140607: Validate returned byte count in RawIOBase.read While `RawIOBase.readinto` should return a count of bytes between 0 and the length of the given buffer, it is not required to. Add validation inside RawIOBase.read that the returned byte count is reasonable. --- Lib/_pyio.py | 2 ++ Lib/test/test_io/test_general.py | 17 +++++++++++++++++ ...25-10-25-21-04-00.gh-issue-140607.oOZGxS.rst | 2 ++ Modules/_io/iobase.c | 13 ++++++++++--- 4 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 9ae72743919a32..8ae2de51184708 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -617,6 +617,8 @@ def read(self, size=-1): n = self.readinto(b) if n is None: return None + if n < 0 or n > len(b): + raise ValueError(f"readinto returned '{n}' outside buffer size '{len(b)}'") del b[n:] return bytes(b) diff --git a/Lib/test/test_io/test_general.py b/Lib/test/test_io/test_general.py index ac9c5a425d7ea2..6a502f14e536ba 100644 --- a/Lib/test/test_io/test_general.py +++ b/Lib/test/test_io/test_general.py @@ -592,6 +592,23 @@ def test_RawIOBase_read(self): self.assertEqual(rawio.read(2), None) self.assertEqual(rawio.read(2), b"") + def test_RawIOBase_read_bounds_checking(self): + # Make sure a `.readinto` call which returns a value oustside + # (0, len(buffer)) raises. + class Misbehaved(io.RawIOBase): + def __init__(self, readinto_return) -> None: + self._readinto_return = readinto_return + def readinto(self, b): + return self._readinto_return + + with self.assertRaises(ValueError) as cm: + Misbehaved(2).read(1) + self.assertEqual(str(cm.exception), "readinto returned '2' oustside buffer size '1'") + self.assertRaises(ValueError, Misbehaved(2147483647).read) + self.assertRaises(ValueError, Misbehaved(sys.maxsize).read) + self.assertRaises(ValueError, Misbehaved(-1).read) + self.assertRaises(ValueError, Misbehaved(-1000).read) + def test_types_have_dict(self): test = ( self.IOBase(), diff --git a/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst b/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst new file mode 100644 index 00000000000000..eb0e542488f8dd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst @@ -0,0 +1,2 @@ +Inside :meth:`io.RawIOBase.read` validate that the count of bytes returned by +:meth:`io.RawIOBase.readinto` is reasonable (inside the provided buffer). diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index acadbcc4d59c38..df1bebb698f6ab 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -939,14 +939,21 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n) return res; } - n = PyNumber_AsSsize_t(res, PyExc_ValueError); + Py_ssize_t bytes_filled = PyNumber_AsSsize_t(res, PyExc_ValueError); Py_DECREF(res); - if (n == -1 && PyErr_Occurred()) { + if (bytes_filled == -1 && PyErr_Occurred()) { Py_DECREF(b); return NULL; } + if (bytes_filled < 0 || bytes_filled > n) { + Py_DECREF(b); + PyErr_Format(PyExc_ValueError, + "readinto returned '%zd' oustside buffer size '%zd'", + bytes_filled, n); + return NULL; + } - res = PyBytes_FromStringAndSize(PyByteArray_AsString(b), n); + res = PyBytes_FromStringAndSize(PyByteArray_AsString(b), bytes_filled); Py_DECREF(b); return res; } From e25ed76c51f643eaec920ba21d8072f0b02875d7 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 26 Oct 2025 12:00:57 -0700 Subject: [PATCH 2/7] Apply suggestion from @ashm-dev Co-authored-by: Shamil --- Lib/test/test_io/test_general.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_io/test_general.py b/Lib/test/test_io/test_general.py index 6a502f14e536ba..157fa984ee71d3 100644 --- a/Lib/test/test_io/test_general.py +++ b/Lib/test/test_io/test_general.py @@ -604,10 +604,9 @@ def readinto(self, b): with self.assertRaises(ValueError) as cm: Misbehaved(2).read(1) self.assertEqual(str(cm.exception), "readinto returned '2' oustside buffer size '1'") - self.assertRaises(ValueError, Misbehaved(2147483647).read) - self.assertRaises(ValueError, Misbehaved(sys.maxsize).read) - self.assertRaises(ValueError, Misbehaved(-1).read) - self.assertRaises(ValueError, Misbehaved(-1000).read) + for bad_size in (2147483647, sys.maxsize, -1, -1000): + with self.assertRaises(ValueError): + Misbehaved(bad_size).read() def test_types_have_dict(self): test = ( From 1e40692dbbe9088d587b087ba7f73434fb7921e4 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 26 Oct 2025 12:03:12 -0700 Subject: [PATCH 3/7] Fix indentation --- Modules/_io/iobase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index df1bebb698f6ab..4f24b3175629b9 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -948,8 +948,8 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n) if (bytes_filled < 0 || bytes_filled > n) { Py_DECREF(b); PyErr_Format(PyExc_ValueError, - "readinto returned '%zd' oustside buffer size '%zd'", - bytes_filled, n); + "readinto returned '%zd' oustside buffer size '%zd'", + bytes_filled, n); return NULL; } From b2e28ddca02d2c9b1ff14c39f0feedeb94bbcb0d Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 26 Oct 2025 12:22:49 -0700 Subject: [PATCH 4/7] oustside->outside, indent from web editor, use self.io --- Lib/test/test_io/test_general.py | 10 +++++----- Modules/_io/iobase.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_io/test_general.py b/Lib/test/test_io/test_general.py index 157fa984ee71d3..57de50b3291d7d 100644 --- a/Lib/test/test_io/test_general.py +++ b/Lib/test/test_io/test_general.py @@ -593,9 +593,9 @@ def test_RawIOBase_read(self): self.assertEqual(rawio.read(2), b"") def test_RawIOBase_read_bounds_checking(self): - # Make sure a `.readinto` call which returns a value oustside + # Make sure a `.readinto` call which returns a value outside # (0, len(buffer)) raises. - class Misbehaved(io.RawIOBase): + class Misbehaved(self.io.RawIOBase): def __init__(self, readinto_return) -> None: self._readinto_return = readinto_return def readinto(self, b): @@ -603,10 +603,10 @@ def readinto(self, b): with self.assertRaises(ValueError) as cm: Misbehaved(2).read(1) - self.assertEqual(str(cm.exception), "readinto returned '2' oustside buffer size '1'") + self.assertEqual(str(cm.exception), "readinto returned '2' outside buffer size '1'") for bad_size in (2147483647, sys.maxsize, -1, -1000): - with self.assertRaises(ValueError): - Misbehaved(bad_size).read() + with self.assertRaises(ValueError): + Misbehaved(bad_size).read() def test_types_have_dict(self): test = ( diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 4f24b3175629b9..95302ad009e37a 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -948,7 +948,7 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n) if (bytes_filled < 0 || bytes_filled > n) { Py_DECREF(b); PyErr_Format(PyExc_ValueError, - "readinto returned '%zd' oustside buffer size '%zd'", + "readinto returned '%zd' outside buffer size '%zd'", bytes_filled, n); return NULL; } From 3d9493afabd54f41f08881d9fe55fc6121303925 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Mon, 27 Oct 2025 10:05:13 -0700 Subject: [PATCH 5/7] Update Lib/_pyio.py Co-authored-by: Victor Stinner --- Lib/_pyio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 8ae2de51184708..423178e87a8684 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -618,7 +618,7 @@ def read(self, size=-1): if n is None: return None if n < 0 or n > len(b): - raise ValueError(f"readinto returned '{n}' outside buffer size '{len(b)}'") + raise ValueError(f"readinto returned {n} outside buffer size {len(b)}") del b[n:] return bytes(b) From 0ab1b2c38bea2552de89b2e408fab9a9f580c894 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Mon, 27 Oct 2025 10:20:58 -0700 Subject: [PATCH 6/7] Remove quotes from error message --- Lib/test/test_io/test_general.py | 2 +- Modules/_io/iobase.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_io/test_general.py b/Lib/test/test_io/test_general.py index 57de50b3291d7d..a1cdd6876c2892 100644 --- a/Lib/test/test_io/test_general.py +++ b/Lib/test/test_io/test_general.py @@ -603,7 +603,7 @@ def readinto(self, b): with self.assertRaises(ValueError) as cm: Misbehaved(2).read(1) - self.assertEqual(str(cm.exception), "readinto returned '2' outside buffer size '1'") + self.assertEqual(str(cm.exception), "readinto returned 2 outside buffer size 1") for bad_size in (2147483647, sys.maxsize, -1, -1000): with self.assertRaises(ValueError): Misbehaved(bad_size).read() diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 95302ad009e37a..e304fc8bee2bea 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -948,7 +948,7 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n) if (bytes_filled < 0 || bytes_filled > n) { Py_DECREF(b); PyErr_Format(PyExc_ValueError, - "readinto returned '%zd' outside buffer size '%zd'", + "readinto returned %zd outside buffer size %zd", bytes_filled, n); return NULL; } From bafc28e7a43322cdb54178aec2a20253ac84bf99 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Mon, 27 Oct 2025 10:34:25 -0700 Subject: [PATCH 7/7] Update Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst Co-authored-by: Victor Stinner --- .../Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst b/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst index eb0e542488f8dd..cc33217c9f563e 100644 --- a/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst +++ b/Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst @@ -1,2 +1,2 @@ -Inside :meth:`io.RawIOBase.read` validate that the count of bytes returned by -:meth:`io.RawIOBase.readinto` is reasonable (inside the provided buffer). +Inside :meth:`io.RawIOBase.read`, validate that the count of bytes returned by +:meth:`io.RawIOBase.readinto` is valid (inside the provided buffer).