From addc87f58c13d53290064d71638492f669580815 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Thu, 30 Jul 2026 13:51:59 +0800 Subject: [PATCH 1/4] gh-148286: Fix undefined behaviour in io.StringIO.read() after an overseek _io_StringIO_read_impl() clamps the read size to zero when the position is past the end of the buffer, but still formed the pointer output = self->buf + self->pos; before returning the empty string. Nothing is read through that pointer, but forming it is undefined behaviour on its own, and because self->buf is a Py_UCS4 *, the byte offset is self->pos * 4: for self->pos == 2**62 - 1 the multiplication wraps and yields a pointer four bytes BELOW self->buf. That is what UBSan reported: io.StringIO("abc").seek(2**62 - 1) followed by .read() stringio.c:352: addition of unsigned offset to 0x... overflowed to 0x... Return the empty string early, exactly as _stringio_readline() already does for the same overseek condition. self->pos += size is a no-op here since size is zero, and ENSURE_REALIZED() is kept ahead of the guard so the state transition and its error path are unchanged. The existing test_io suite already covers this path, which is how UBSan found it. The Modules/_io/stringio.c entry in Tools/ubsan/suppressions.txt is no longer needed; with it removed, test_io passes under UBSAN_OPTIONS=halt_on_error=1, where before it aborts at stringio.c:352. --- .../2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst | 7 +++++++ Modules/_io/stringio.c | 11 +++++++++++ Tools/ubsan/suppressions.txt | 3 --- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst diff --git a/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst new file mode 100644 index 000000000000000..749c3de18f5ae9e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst @@ -0,0 +1,7 @@ +Fix undefined behaviour in :meth:`!io.StringIO.read` after an overseek. +Reading from a position past the end of the buffer formed the pointer +``self->buf + self->pos`` before returning the empty string; for a large +enough position the arithmetic wrapped and produced a pointer below the +buffer. :class:`io.StringIO` now returns the empty string early, as +``_stringio_readline()`` already did, which removes the +``Modules/_io/stringio.c`` entry from ``Tools/ubsan/suppressions.txt``. diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 27605b4c44239e9..ff74acee0bbc8f9 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -349,6 +349,17 @@ _io_StringIO_read_impl(stringio *self, Py_ssize_t size) } ENSURE_REALIZED(self); + + /* In case of overseek, return the empty string, as _stringio_readline() + does. `size` is already clamped to 0 here, so nothing would be read + through the pointer -- but forming `self->buf + self->pos` when + `self->pos` is past the end of the buffer is undefined behaviour on + its own, and for a large enough `self->pos` the multiplication by + sizeof(Py_UCS4) wraps and yields a pointer below `self->buf`. */ + if (self->pos >= self->string_size) { + return Py_GetConstant(Py_CONSTANT_EMPTY_STR); + } + output = self->buf + self->pos; self->pos += size; return PyUnicode_FromKindAndData(PyUnicode_4BYTE_KIND, output, size); diff --git a/Tools/ubsan/suppressions.txt b/Tools/ubsan/suppressions.txt index 2b9ad49ebf1eefb..b22663e47759e9b 100644 --- a/Tools/ubsan/suppressions.txt +++ b/Tools/ubsan/suppressions.txt @@ -14,6 +14,3 @@ shift-base:Modules/_ctypes/cfield.c # Modules/_ctypes/cfield.c:640:1: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int' signed-integer-overflow:Modules/_ctypes/cfield.c - -# Modules/_io/stringio.c:350:24: runtime error: addition of unsigned offset to 0x7fd01ec25850 overflowed to 0x7fd01ec2584c -pointer-overflow:Modules/_io/stringio.c From 0d659fac09ddfac7a7f73507484729d93dd0fdc3 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Sat, 1 Aug 2026 17:09:19 +0800 Subject: [PATCH 2/4] Add a regression test for large overseeks; trim the NEWS entry Review feedback: picnixz asked for a regression test, StanFromIreland for the NEWS entry to drop implementation and infrastructure detail. The existing test_overseek only seeks one past the end, which is why it never exercised this: the pointer arithmetic only wraps for positions above PY_SSIZE_T_MAX // 4. The new test covers 2**31, 2**62 and sys.maxsize, and sits in MemoryTestMixin so it runs for BytesIO as well as StringIO, in both the C and pure-Python implementations. Verified that all four already satisfy it, which is the point -- it pins the documented behaviour rather than detecting the UB. NEWS entry cut to the user-visible statement. --- Lib/test/test_io/test_memoryio.py | 17 +++++++++++++++++ ...26-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst | 9 ++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_io/test_memoryio.py b/Lib/test/test_io/test_memoryio.py index f53525d77b7dbaf..ead4a6636b9149f 100644 --- a/Lib/test/test_io/test_memoryio.py +++ b/Lib/test/test_io/test_memoryio.py @@ -341,6 +341,23 @@ def test_overseek(self): memio.write(buf) self.assertEqual(memio.getvalue(), buf + self.buftype('\0') + buf) + def test_overseek_far_past_end(self): + # gh-148286: the existing test_overseek only seeks one past the + # end. StringIO.read() formed ``self->buf + self->pos`` before + # returning the empty string, which is undefined behaviour for + # any overseek -- and for a position above PY_SSIZE_T_MAX // 4 + # the multiplication by sizeof(Py_UCS4) wrapped, yielding a + # pointer below the buffer. Only large offsets exercise that. + buf = self.buftype("1234567890") + for pos in (2 ** 31, 2 ** 62, sys.maxsize): + with self.subTest(pos=pos): + memio = self.ioclass(buf) + self.assertEqual(memio.seek(pos), pos) + self.assertEqual(memio.read(), self.EOF) + self.assertEqual(memio.read(10), self.EOF) + self.assertEqual(memio.tell(), pos) + self.assertEqual(memio.getvalue(), buf) + def test_tell(self): buf = self.buftype("1234567890") memio = self.ioclass(buf) diff --git a/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst index 749c3de18f5ae9e..a631356609617c2 100644 --- a/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst +++ b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst @@ -1,7 +1,2 @@ -Fix undefined behaviour in :meth:`!io.StringIO.read` after an overseek. -Reading from a position past the end of the buffer formed the pointer -``self->buf + self->pos`` before returning the empty string; for a large -enough position the arithmetic wrapped and produced a pointer below the -buffer. :class:`io.StringIO` now returns the empty string early, as -``_stringio_readline()`` already did, which removes the -``Modules/_io/stringio.c`` entry from ``Tools/ubsan/suppressions.txt``. +Fix undefined behaviour in :meth:`io.StringIO.read` when reading from a +position past the end of the buffer. From 9d3ba83929666a0f21bbe06fccd3de757155ee08 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Sat, 1 Aug 2026 17:36:45 +0800 Subject: [PATCH 3/4] Drop the added test; the existing testSeek already covers this Verified under UBSan with CI's own configuration (--with-undefined-behavior-sanitizer --with-strict-overflow, clang): CStringIOTest.testSeek already triggers the report, because it does seek(sys.maxsize) followed by read(4), which is exactly the case where the pointer arithmetic wraps. That is what produced the diagnostic recorded in the suppressions file. My added test was therefore redundant for detection: small overseeks (11, 100, 2**31) produce no report at all, and the large offsets it used were already covered. Removed rather than kept as noise. Left a comment on testSeek instead, since its name gives no hint that those two lines guard a UB fix, and trimming them would silently remove the coverage. --- Lib/test/test_io/test_memoryio.py | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_io/test_memoryio.py b/Lib/test/test_io/test_memoryio.py index ead4a6636b9149f..649b8677d5d1202 100644 --- a/Lib/test/test_io/test_memoryio.py +++ b/Lib/test/test_io/test_memoryio.py @@ -54,6 +54,9 @@ def testSeek(self): self.assertEqual(buf[3:], bytesIo.read()) self.assertRaises(TypeError, bytesIo.seek, 0.0) + # gh-148286: seeking this far past the end and then reading is + # what exercises the overseek path where the pointer arithmetic + # wraps. Under UBSan this is the case that reports, so keep it. self.assertEqual(sys.maxsize, bytesIo.seek(sys.maxsize)) self.assertEqual(self.EOF, bytesIo.read(4)) @@ -341,23 +344,6 @@ def test_overseek(self): memio.write(buf) self.assertEqual(memio.getvalue(), buf + self.buftype('\0') + buf) - def test_overseek_far_past_end(self): - # gh-148286: the existing test_overseek only seeks one past the - # end. StringIO.read() formed ``self->buf + self->pos`` before - # returning the empty string, which is undefined behaviour for - # any overseek -- and for a position above PY_SSIZE_T_MAX // 4 - # the multiplication by sizeof(Py_UCS4) wrapped, yielding a - # pointer below the buffer. Only large offsets exercise that. - buf = self.buftype("1234567890") - for pos in (2 ** 31, 2 ** 62, sys.maxsize): - with self.subTest(pos=pos): - memio = self.ioclass(buf) - self.assertEqual(memio.seek(pos), pos) - self.assertEqual(memio.read(), self.EOF) - self.assertEqual(memio.read(10), self.EOF) - self.assertEqual(memio.tell(), pos) - self.assertEqual(memio.getvalue(), buf) - def test_tell(self): buf = self.buftype("1234567890") memio = self.ioclass(buf) From 047e967405b4e3371592944aa452a24efb52e8c1 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Sat, 1 Aug 2026 20:31:03 +0800 Subject: [PATCH 4/4] Restore the suppressed reference in the NEWS entry Dropping the '!' from :meth:`!io.StringIO.read` while trimming the entry broke the docs build: there is no doc target for io.StringIO.read, so nitpicky mode reports 'py:meth reference target not found'. The original had the bang for exactly that reason. Every build check passed; Docs was the only real failure of the three (the other two are the unresolved-review gate and the aggregate that depends on it). --- .../next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst index a631356609617c2..9b2c98a5ba74a6f 100644 --- a/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst +++ b/Misc/NEWS.d/next/Library/2026-07-30-12-05-00.gh-issue-148286.Bv3Nq8.rst @@ -1,2 +1,2 @@ -Fix undefined behaviour in :meth:`io.StringIO.read` when reading from a +Fix undefined behaviour in :meth:`!io.StringIO.read` when reading from a position past the end of the buffer.