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

bast0006 · 2025-04-10T20:47:17Z

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

…ay allocation during item assignment

python-cla-bot · 2025-04-10T20:47:21Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2025-04-10T20:47:22Z

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.

bast0006 · 2025-04-10T22:04:42Z

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

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

Lib/test/test_bytes.py

…te test

bast0006 · 2025-05-17T23:23:46Z

Done

Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-18-22-12.gh-issue-91153.ioA_83.rst

Lib/test/test_bytes.py

Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-20-56-05.gh-issue-91153.afgtG2.rst

Objects/bytearrayobject.c

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

ZeroIntensity · 2025-06-03T09:55:27Z

Lib/test/test_bytes.py

                self._testlimitedcapi.sequence_setitem(b, 0, Boom())

+    def test_mutating_index_inbounds(self):
+        # See gh-91153


Could you add a brief description of the issue?

Objects/bytearrayobject.c

picnixz

2 NITS and I'll merge.

Objects/bytearrayobject.c

miss-islington-app · 2025-07-12T13:37:56Z

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

…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 · 2025-07-12T13:38:03Z

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 · 2025-07-12T13:38:04Z

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

… ...)` 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 · 2025-07-12T13:46:31Z

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

… when `ind.__index__` has side-effects (GH-132379) (#136582) (cherry picked from commit 5e1e21d) Co-authored-by: Bast <52266665+bast0006@users.noreply.github.com>

… 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>

…when `ind.__index__` has side-effects (python#132379) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

… ...)` 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>

bast0006 added 2 commits March 29, 2025 07:36

pythonGH-91153: Handle _getbytevalue potentially invalidating bytearr…

71ae212

…ay allocation during item assignment

pythonGH-91153: Add additional cases to test_mutating_index

b2f7df5

bedevere-app bot added the awaiting review label Apr 10, 2025

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

picnixz self-requested a review May 17, 2025 11:00

picnixz reviewed May 17, 2025

View reviewed changes

Lib/test/test_bytes.py Outdated Show resolved Hide resolved

Lib/test/test_bytes.py Outdated Show resolved Hide resolved

bast0006 added 2 commits May 17, 2025 18:13

pythonGH-91153: Move mutating index bytearray test to it's own separa…

bb96c2a

…te test

pythonGH-91153: add news entry

d29422b

bast0006 requested a review from picnixz May 17, 2025 23:25

ZeroIntensity reviewed May 18, 2025

View reviewed changes

Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-18-22-12.gh-issue-91153.ioA_83.rst Outdated Show resolved Hide resolved

Lib/test/test_bytes.py Show resolved Hide resolved

ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 18, 2025

Nit cleanup

d3d1974

bast0006 force-pushed the bast0006/gh-91153 branch from 3c1de75 to d3d1974 Compare May 18, 2025 03:53

picnixz reviewed Jun 2, 2025

View reviewed changes

pythonGH-91153: Fixup comments and NEWS entry

18e6597

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

ZeroIntensity approved these changes Jun 3, 2025

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Jun 3, 2025

bast0006 force-pushed the bast0006/gh-91153 branch from 5853b5c to 50b7998 Compare June 3, 2025 20:53

pythonGH-91153: Add short clarifying comments to test

4b7ec9c

bast0006 force-pushed the bast0006/gh-91153 branch from 50b7998 to 4b7ec9c Compare June 3, 2025 20:57

Merge branch 'main' into bast0006/pythongh-91153

9b28503

picnixz approved these changes Jul 12, 2025

View reviewed changes

Objects/bytearrayobject.c Outdated Show resolved Hide resolved

Objects/bytearrayobject.c Outdated Show resolved Hide resolved

bedevere-app bot added awaiting merge and removed awaiting core review labels Jul 12, 2025

Apply suggestions from code review

970c1ad

picnixz enabled auto-merge (squash) July 12, 2025 13:15

picnixz merged commit 5e1e21d into python:main Jul 12, 2025
41 checks passed

bedevere-app bot removed the awaiting merge label Jul 12, 2025

miss-islington-app bot assigned picnixz Jul 12, 2025

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

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

Uh oh!

Conversation

bast0006 commented Apr 10, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot bot commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented Apr 10, 2025

Uh oh!

bast0006 commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bast0006 commented May 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

bast0006 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington-app bot commented Jul 12, 2025

Uh oh!

miss-islington-app bot commented Jul 12, 2025

Uh oh!

bedevere-app bot commented Jul 12, 2025

Uh oh!

bedevere-app bot commented Jul 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bast0006 commented Apr 10, 2025 •

edited by bedevere-app bot

Loading

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

edited

Loading

bast0006 commented Apr 10, 2025 •

edited

Loading

picnixz left a comment •

edited

Loading