GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — 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

Conversation

@bast0006

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

  • Issue: UAF when writing to a bytearray with an element implementing __index__ with side-effects #91153

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
bast0006 added 2 commits March 29, 2025 07:36
@bast0006
pythonGH-91153: Handle _getbytevalue potentially invalidating bytearr…
71ae212 
…ay allocation during item assignment
@bast0006
pythonGH-91153: Add additional cases to test_mutating_index
b2f7df5
@python-cla-bot

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed
bast0006 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting review label Apr 10, 2025
@bedevere-app

bedevere-app Bot commented Apr 10, 2025

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot mentioned this pull request Apr 10, 2025
UAF when writing to a bytearray with an element implementing __index__ with side-effects #91153
Closed
@bast0006

bast0006 commented Apr 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
picnixz self-requested a review May 17, 2025 11:00
picnixz
picnixz reviewed May 17, 2025
View reviewed changes

@picnixz picnixz left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
bast0006 added 2 commits May 17, 2025 18:13
@bast0006
pythonGH-91153: Move mutating index bytearray test to it's own separa…
bb96c2a 
…te test
@bast0006
pythonGH-91153: add news entry
d29422b
@bast0006

bast0006 commented May 17, 2025

Copy link
Copy Markdown
Contributor Author

Done
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity ZeroIntensity added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels May 18, 2025
@bast0006
Nit cleanup
d3d1974
@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 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-20-56-05.gh-issue-91153.afgtG2.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bast0006 @picnixz
pythonGH-91153: Fixup comments and NEWS entry
18e6597 
Co-authored-by: Bénédikt Tran <[email protected]>
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

@ZeroIntensity ZeroIntensity Jun 3, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bast0006 bast0006 Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Jun 3, 2025
@bast0006
bast0006 force-pushed the bast0006/gh-91153 branch from 5853b5c to 50b7998 Compare June 3, 2025 20:53
@bast0006
pythonGH-91153: Add short clarifying comments to test
4b7ec9c
@bast0006
bast0006 force-pushed the bast0006/gh-91153 branch from 50b7998 to 4b7ec9c Compare June 3, 2025 20:57
@picnixz
Merge branch 'main' into bast0006/pythongh-91153
9b28503
picnixz
picnixz approved these changes Jul 12, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jul 12, 2025
@picnixz
Apply suggestions from code review
970c1ad
@picnixz
picnixz enabled auto-merge (squash) July 12, 2025 13:15
@picnixz
picnixz merged commit 5e1e21d into python:main Jul 12, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 12, 2025
@miss-islington-app

miss-islington-app Bot commented Jul 12, 2025

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@miss-islington-app

miss-islington-app Bot commented Jul 12, 2025

Copy link
Copy Markdown

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Jul 12, 2025

Copy link
Copy Markdown

GH-136581 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned picnixz Jul 12, 2025
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security 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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Jul 12, 2025

Copy link
Copy Markdown

GH-136582 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security 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 <[email protected]>
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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@picnixz picnixz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@bast0006 @picnixz @ZeroIntensity
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.