gh-140607: Validate returned byte count in RawIOBase.read #140611

cmaloney · 2025-10-26T04:29:48Z

While readinto implementations should return a count of bytes between 0 and the length of the given buffer, they are not required to. Add validation inside RawIOBase.read that the returned byte count is reasonable.

Issue: heap-buffer-overflow in _io__RawIOBase_read #140607

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.

cmaloney · 2025-10-26T05:46:49Z

It looks like the IOBase C implementation has never checked range here. I think it's reasonable to back port to bugfix releases as a ValueError could be raised in other cases (not adding a new exception; just more cases) and a bad value allows access to unallocated memory. Cherry picks will likely need to be manual to cross the I/O test refactoring.

The _pyio change could also potentially use a n = n.__index__() to make sure the .readinto return is an integer at that point; not sure it's worth adding though.

cc: @vstinner

Lib/test/test_io/test_general.py

Modules/_io/iobase.c

Lib/test/test_io/test_general.py

Co-authored-by: Shamil <[email protected]>

Lib/test/test_io/test_general.py

Modules/_io/iobase.c

Lib/_pyio.py

efimov-mikhail

LGTM

Lib/_pyio.py

ashm-dev · 2025-10-27T16:52:28Z

Modules/_io/iobase.c

+    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' outside buffer size '%zd'",
+                     bytes_filled, n);
+        return NULL;
+    }


if (bytes_filled == -1 && PyErr_Occurred()) goto error; if (bytes_filled < 0 || bytes_filled > n) { PyErr_Format(PyExc_ValueError, "readinto returned '%zd' outside buffer size '%zd'", bytes_filled, n); goto error; } error: Py_DECREF(b); return NULL;

maybe do it this way to remove duplication and improve readability

I have a personal preference not to use gotos although agree this function is one where Py_DECREF(b); return res; would deduplicate and reduce lines of code.

To do that the Py_DECREF(res); would need to turn into Py_CLEAR(res) (so res is set to NULL) + a couple cases earlier could use it as well... Not planning to do that but can if there's a strong press to.

Just kidding. In general, I also like using goto in C to factorize code to cleanup resources before exit. Here it's just a matter of taste. I'm fine with the current code 😀

Co-authored-by: Victor Stinner <[email protected]>

vstinner · 2025-10-27T17:26:14Z

I think it's reasonable to back port to bugfix releases as a ValueError could be raised in other cases (not adding a new exception; just more cases) and a bad value allows access to unallocated memory.

I prefer to not backport the change. Existing code may just work by luck and this change making Python more strict can break it.

vstinner

LGTM

Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst

…OZGxS.rst Co-authored-by: Victor Stinner <[email protected]>

ashm-dev

LGTM

miss-islington-app · 2025-10-28T17:01:31Z

Thanks @cmaloney for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2025-10-28T17:01:31Z

Thanks @cmaloney for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington-app · 2025-10-28T17:01:35Z

Sorry, @cmaloney and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0f0a362768aecb4c791724cce486d8317533a94d 3.13

miss-islington-app · 2025-10-28T17:01:39Z

Sorry, @cmaloney and @vstinner, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0f0a362768aecb4c791724cce486d8317533a94d 3.14

vstinner · 2025-10-28T17:03:00Z

I prefer to not backport the change. Existing code may just work by luck and this change making Python more strict can break it.

I read again the issue and I missed the AddressSanitizer report in the issue. It changed my mind and I know agree that it's better to backport this change.

@cmaloney: Can you try to backport the change to 3.14? The automated backport failed.

cmaloney · 2025-10-28T19:31:17Z

👍 will work on backports today

bedevere-app · 2025-10-28T21:50:45Z

GH-140728 is a backport of this pull request to the 3.14 branch.

pythonGH-140611) 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 valid. (cherry picked from commit 0f0a362) Co-authored-by: Cody Maloney <[email protected]> Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

bedevere-app · 2025-10-28T22:00:50Z

GH-140730 is a backport of this pull request to the 3.13 branch.

…140611) (#140728) * [3.14] gh-140607: Validate returned byte count in RawIOBase.read (GH-140611) 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 valid. (cherry picked from commit 0f0a362) Co-authored-by: Cody Maloney <[email protected]> Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]> * fixup: Use older attribute name --------- Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

…140611) (#140730) * [3.13] gh-140607: Validate returned byte count in RawIOBase.read (GH-140611) 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 valid. (cherry picked from commit 0f0a362) Co-authored-by: Cody Maloney <[email protected]> Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]> * fixup: Use older attribute name --------- Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

pythongh-140607: Validate returned byte count in RawIOBase.read

f25ce8a

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.

cmaloney added topic-IO needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Oct 26, 2025

bedevere-app bot mentioned this pull request Oct 26, 2025

heap-buffer-overflow in _io__RawIOBase_read #140607

Closed

bedevere-app bot added the awaiting review label Oct 26, 2025

efimov-mikhail reviewed Oct 26, 2025

View reviewed changes

Lib/test/test_io/test_general.py Show resolved Hide resolved

Modules/_io/iobase.c Outdated Show resolved Hide resolved

ashm-dev suggested changes Oct 26, 2025

View reviewed changes

Lib/test/test_io/test_general.py Outdated Show resolved Hide resolved

bedevere-app bot added awaiting core review and removed awaiting review labels Oct 26, 2025

cmaloney and others added 2 commits October 26, 2025 12:00

Apply suggestion from @ashm-dev

e25ed76

Co-authored-by: Shamil <[email protected]>

Fix indentation

1e40692

cmaloney requested a review from ashm-dev October 26, 2025 19:03

efimov-mikhail reviewed Oct 26, 2025

View reviewed changes

Lib/test/test_io/test_general.py Outdated Show resolved Hide resolved

ashm-dev approved these changes Oct 26, 2025

View reviewed changes

efimov-mikhail reviewed Oct 26, 2025

View reviewed changes

Modules/_io/iobase.c Outdated Show resolved Hide resolved

efimov-mikhail reviewed Oct 26, 2025

View reviewed changes

Lib/_pyio.py Outdated Show resolved Hide resolved

oustside->outside, indent from web editor, use self.io

b2e28dd

efimov-mikhail approved these changes Oct 26, 2025

View reviewed changes

efimov-mikhail requested a review from vstinner October 26, 2025 20:07

vstinner reviewed Oct 27, 2025

View reviewed changes

Lib/_pyio.py Outdated Show resolved Hide resolved

ashm-dev reviewed Oct 27, 2025

View reviewed changes

cmaloney and others added 2 commits October 27, 2025 10:05

Update Lib/_pyio.py

3d9493a

Co-authored-by: Victor Stinner <[email protected]>

Remove quotes from error message

0ab1b2c

vstinner removed needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Oct 27, 2025

vstinner approved these changes Oct 27, 2025

View reviewed changes

Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.oOZGxS.rst Outdated Show resolved Hide resolved

bedevere-app bot removed the awaiting core review label Oct 27, 2025

bedevere-app bot added the awaiting merge label Oct 27, 2025

Update Misc/NEWS.d/next/Library/2025-10-25-21-04-00.gh-issue-140607.o…

bafc28e

…OZGxS.rst Co-authored-by: Victor Stinner <[email protected]>

ashm-dev approved these changes Oct 27, 2025

View reviewed changes

vstinner enabled auto-merge (squash) October 27, 2025 17:43

vstinner merged commit 0f0a362 into python:main Oct 27, 2025
45 checks passed

bedevere-app bot removed the awaiting merge label Oct 27, 2025

cmaloney deleted the gh-140607 branch October 27, 2025 18:22

vstinner added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Oct 28, 2025

miss-islington-app bot assigned vstinner Oct 28, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Oct 28, 2025

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 28, 2025

Uh oh!

gh-140607: Validate returned byte count in RawIOBase.read #140611

gh-140607: Validate returned byte count in RawIOBase.read #140611

Conversation

cmaloney commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmaloney commented Oct 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

efimov-mikhail left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashm-dev Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

cmaloney Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 27, 2025

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashm-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app bot commented Oct 28, 2025

Uh oh!

miss-islington-app bot commented Oct 28, 2025

Uh oh!

miss-islington-app bot commented Oct 28, 2025

Uh oh!

miss-islington-app bot commented Oct 28, 2025

Uh oh!

vstinner commented Oct 28, 2025

Uh oh!

cmaloney commented Oct 28, 2025

Uh oh!

bedevere-app bot commented Oct 28, 2025

Uh oh!

bedevere-app bot commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cmaloney commented Oct 26, 2025 •

edited

Loading