Skip to content

gh-140594: Fix an out of bounds read when feeding NUL byte to PyOS_StdioReadline#140910

Merged
vstinner merged 19 commits intopython:mainfrom
ashm-dev:asan
Mar 12, 2026
Merged

gh-140594: Fix an out of bounds read when feeding NUL byte to PyOS_StdioReadline#140910
vstinner merged 19 commits intopython:mainfrom
ashm-dev:asan

Conversation

@ashm-dev
Copy link
Contributor

@ashm-dev ashm-dev commented Nov 2, 2025

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests.

@bedevere-app
Copy link

bedevere-app bot commented Nov 2, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

@picnixz picnixz changed the title gh-140594: Fix buffer underflow in PyOS_StdioReadline on null bytes gh-140594: Fix buffer overflow when feeding NULL bytes to PyOS_StdioReadline Nov 2, 2025
@picnixz
Copy link
Member

picnixz commented Nov 2, 2025

FTR, a buffer underflow is something entirely different.

@ashm-dev
Copy link
Contributor Author

ashm-dev commented Nov 2, 2025

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Nov 2, 2025

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from picnixz November 2, 2025 16:45
@bedevere-app
Copy link

bedevere-app bot commented Nov 2, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ashm-dev
Copy link
Contributor Author

ashm-dev commented Nov 2, 2025

I have made the requested changes; please review again

picnixz
picnixz previously requested changes Nov 2, 2025
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Popen seems an overkill.

@bedevere-app
Copy link

bedevere-app bot commented Nov 2, 2025

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

Copy link
Contributor

@sergey-miryanov sergey-miryanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@efimov-mikhail efimov-mikhail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ashm-dev
Copy link
Contributor Author

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Mar 11, 2026

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@vstinner vstinner dismissed picnixz’s stale review March 12, 2026 10:23

@picnixz reviews have been addressed.

@vstinner
Copy link
Member

Aha, I reproduced the ASAN error using ./configure --with-address-sanitizer CC=clang LD=clang and #140594 (comment) reproducer.

$ ./python bug.py 
==3640856==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7ca08c6474ff at pc 0x00000059d67e bp 0x7fff55cd4c50 sp 0x7fff55cd4c48
READ of size 1 at 0x7ca08c6474ff thread T0
    #0 0x00000059d67d in PyOS_StdioReadline /home/vstinner/python/main/Parser/myreadline.c:347:14
    #1 0x00000059d8bf in PyOS_Readline /home/vstinner/python/main/Parser/myreadline.c:411:14
    ...

0x7ca08c6474ff is located 1 bytes before 100-byte region [0x7ca08c647500,0x7ca08c647564)
allocated by thread T0 here:
    #0 0x0000004a8d00 in realloc (/home/vstinner/python/main/python+0x4a8d00) (BuildId: 764499fff4f990ace65f94f78d08a62da34d9197)
    #1 0x00000059d536 in PyOS_StdioReadline /home/vstinner/python/main/Parser/myreadline.c:327:22
    ...
...

Using the ASAN build without the fix, the new test fails as expected:

FAIL: test_null_byte_in_interactive_mode (test.test_cmd_line.CmdLineTest.test_null_byte_in_interactive_mode)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/test/test_cmd_line.py", line 209, in test_null_byte_in_interactive_mode
    self.assertEqual(proc.returncode, 0)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
AssertionError: 1 != 0

Using the ASAN build with the fix, the new test pass as expected.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vstinner vstinner enabled auto-merge (squash) March 12, 2026 10:41
@vstinner vstinner merged commit 86a0756 into python:main Mar 12, 2026
53 of 54 checks passed
@ashm-dev ashm-dev deleted the asan branch March 12, 2026 10:47
@vstinner vstinner added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Mar 12, 2026
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 12, 2026
…yOS_StdioReadline() (pythonGH-140910)

(cherry picked from commit 86a0756)

Co-authored-by: Shamil <ashm.tech@proton.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 12, 2026
…yOS_StdioReadline() (pythonGH-140910)

(cherry picked from commit 86a0756)

Co-authored-by: Shamil <ashm.tech@proton.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Mar 12, 2026

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

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Mar 12, 2026
@bedevere-app
Copy link

bedevere-app bot commented Mar 12, 2026

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

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Mar 12, 2026
@vstinner vstinner changed the title gh-140594: Fix buffer overflow when feeding NULL bytes to PyOS_StdioReadline gh-140594: Fix an out of bounds read when feeding NUL byte to PyOS_StdioReadline Mar 12, 2026
@vstinner
Copy link
Member

Thanks @ashm-dev for the fix, I merged your PR and backported the fix to 3.13 and 3.14 branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants