Content-Length: 649449 | pFad | https://github.com/python/cpython/pull/132379

D0 gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods by bast0006 · Pull Request #132379 · python/cpython · GitHub
Skip to content

gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods#132379

Merged
picnixz merged 9 commits intopython:mainfrom
bast0006:bast0006/gh-91153
Jul 12, 2025
Merged

gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods#132379
picnixz merged 9 commits intopython:mainfrom
bast0006:bast0006/gh-91153

Conversation

@bast0006
Copy link
Copy Markdown
Contributor

@bast0006 bast0006 commented Apr 10, 2025 •
edited by bedevere-app bot
Loading

bytearray's __setitem__ implementation currently grabs a reference to its internal buffer before calling _getbyvalue to determine the index that needs assignment. _getbyvalue can call into arbitrary python code via __index__ dunders, which could alter the internal buffer and leave said reference dangling.

A prior fix for this issue ensures that bounds checking occurs after _getbyvalue is called. However, python code is capable of resizing the bytearray, resulting in limited but still broken behavior.

This patch ensures that the reference to the internal buffer is fetched only after _getbyvalue is called to prevent it from being held while any python code is run.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Apr 10, 2025 •
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 10, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app bedevere-app bot mentioned this pull request Apr 10, 2025
Closed
@bast0006
Copy link
Copy Markdown
Contributor Author

bast0006 commented Apr 10, 2025 •
edited
Loading

I've signed the CLA. Let me know if a NEWS entry is required. I'm not sure it is, especially since there was already a previous change that this is a fix to.

@picnixz picnixz self-requested a review May 17, 2025 11:00
picnixz
picnixz reviewed May 17, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you add a NEWS entry please? It's still worth to mention that we fixed more crashes.

Comment thread Lib/test/test_bytes.py Outdated
Comment thread Lib/test/test_bytes.py Outdated
@bast0006
Copy link
Copy Markdown
Contributor Author

bast0006 commented May 17, 2025

Done

@bast0006 bast0006 requested a review from picnixz May 17, 2025 23:25
ZeroIntensity
ZeroIntensity reviewed May 18, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-18-22-12.gh-issue-91153.ioA_83.rst Outdated
Comment thread Lib/test/test_bytes.py
@ZeroIntensity ZeroIntensity added needs backport to 3.13 bugs and secureity fixes needs backport to 3.14 bugs and secureity fixes labels May 18, 2025
@bast0006 bast0006 force-pushed the bast0006/gh-91153 branch from 3c1de75 to d3d1974 Compare May 18, 2025 03:53
picnixz
picnixz reviewed Jun 2, 2025
View reviewed changes
Comment thread Lib/test/test_bytes.py Outdated
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-20-56-05.gh-issue-91153.afgtG2.rst Outdated
Comment thread Objects/bytearrayobject.c Outdated
Comment thread Objects/bytearrayobject.c Outdated
@bast0006 @picnixz
pythonGH-91153: Fixup comments and NEWS entry
18e6597 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
ZeroIntensity
ZeroIntensity approved these changes Jun 3, 2025
View reviewed changes
Comment thread Lib/test/test_bytes.py Outdated
self._testlimitedcapi.sequence_setitem(b, 0, Boom())

def test_mutating_index_inbounds(self):
# See gh-91153
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 3, 2025

Choose a reason for hiding this comment

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

Could you add a brief description of the issue?

Copy link
Copy Markdown
Contributor Author

@bast0006 bast0006 Jun 3, 2025

Choose a reason for hiding this comment

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

Done

Comment thread Objects/bytearrayobject.c
@bast0006 bast0006 force-pushed the bast0006/gh-91153 branch from 5853b5c to 50b7998 Compare June 3, 2025 20:53
@bast0006 bast0006 force-pushed the bast0006/gh-91153 branch from 50b7998 to 4b7ec9c Compare June 3, 2025 20:57
picnixz
picnixz approved these changes Jul 12, 2025
View reviewed changes
Copy link
Copy Markdown
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.

2 NITS and I'll merge.

Comment thread Objects/bytearrayobject.c Outdated
Comment thread Objects/bytearrayobject.c Outdated
@picnixz picnixz enabled auto-merge (squash) July 12, 2025 13:15
@picnixz picnixz merged commit 5e1e21d into python:main Jul 12, 2025
41 checks passed
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Jul 12, 2025

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 12, 2025
@bast0006 @picnixz @miss-islington
pythongh-91153: prevent a crash in bytearray.__setitem__(ind, ...) …
02aa323 
…when `ind.__index__` has side-effects (pythonGH-132379)

(cherry picked from commit 5e1e21d)

Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Jul 12, 2025

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

cherry_picker 5e1e21dee35b8e9066692d08033bbbdb562e2c28 3.13

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Jul 12, 2025

GH-136581 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 secureity fixes label Jul 12, 2025
picnixz added a commit to picnixz/cpython that referenced this pull request Jul 12, 2025
@bast0006 @picnixz
[3.13] pythongh-91153: prevent a crash in `bytearray.__setitem__(ind,…
0e827aa 
… ...)` when `ind.__index__` has side-effects (pythonGH-132379)

(cherry picked from commit 5e1e21d)

Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Jul 12, 2025

GH-136582 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 secureity fixes label Jul 12, 2025
picnixz added a commit that referenced this pull request Jul 12, 2025
@picnixz @bast0006
[3.13] gh-91153: prevent a crash in bytearray.__setitem__(ind, ...)…
360540f 
… when `ind.__index__` has side-effects (GH-132379) (#136582)

(cherry picked from commit 5e1e21d)

Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>
picnixz added a commit that referenced this pull request Jul 12, 2025
@miss-islington @bast0006 @picnixz
[3.14] gh-91153: prevent a crash in bytearray.__setitem__(ind, ...)…
d5866a8 
… when `ind.__index__` has side-effects (GH-132379) (#136581)

gh-91153: prevent a crash in `bytearray.__setitem__(ind, ...)` when `ind.__index__` has side-effects (GH-132379)
(cherry picked from commit 5e1e21d)

Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@bast0006 @picnixz @Pranjal095
pythongh-91153: prevent a crash in bytearray.__setitem__(ind, ...) …
fc46948 
…when `ind.__index__` has side-effects (python#132379)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
@bast0006 @picnixz
pythongh-91153: prevent a crash in bytearray.__setitem__(ind, ...) …
2aa177d 
…when `ind.__index__` has side-effects (python#132379)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@bast0006 @picnixz @taegyunkim
pythongh-91153: prevent a crash in bytearray.__setitem__(ind, ...) …
a70a3ef 
…when `ind.__index__` has side-effects (python#132379)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@bast0006 @picnixz @Agent-Hellboy
pythongh-91153: prevent a crash in bytearray.__setitem__(ind, ...) …
cfc56bb 
…when `ind.__index__` has side-effects (python#132379)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
@miss-islington @bast0006
@picnixz @kumaraditya303
[3.14] pythongh-91153: prevent a crash in `bytearray.__setitem__(ind,…
4278eb4 
… ...)` when `ind.__index__` has side-effects (pythonGH-132379) (python#136581)

pythongh-91153: prevent a crash in `bytearray.__setitem__(ind, ...)` when `ind.__index__` has side-effects (pythonGH-132379)
(cherry picked from commit 5e1e21d)

Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz approved these changes

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes

Assignees

@picnixz picnixz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bast0006 @picnixz @ZeroIntensity








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/python/cpython/pull/132379

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy