gh-101765: Fix SystemError / segmentation fault in iter `reduce` when internal access of `builtins.dict` exhausts the iterator by ionite34 · Pull Request #101769 · python/cpython · GitHub

Summary

Fixes potential segmentation fault or SystemError when __reduce__ is called on iter objects when the key of the hash of "iter" within __builtins__.__dict__ is a custom object that executes arbitrary code within __eq__, mutating the iter object and causing illegal memory access or SystemError (based on C argument evaluation order, which is undefined behavior).

Affected methods

iterobject.c
- iter_reduce
- calliter_reduce
listobject.c
- listiter_reduce_general
bytearrayobject.c
- bytearrayiter_reduce
bytesobject.c
- striter_reduce
tupleobject.c
- tupleiter_reduce
genericaliasobject.c
- ga_iter_reduce

This PR also fixes a compounded issue where currently genericaliasobject.ga_iter_reduce does not handle empty iterators at all and has no NULL check

Python 3.11.0 (main, Oct 26 2022, 10:14:06) [GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> x = tuple[int]
>>>
>>> it = iter(x)
>>> _ = list(it)
>>>
>>> it.__reduce__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
SystemError: NULL object passed to Py_BuildValue

Along with moving the evaluation of _PyEval_GetBuiltin for potential side effects, this also adds handling of the NULL case (like the other iter_reduce functions have:

Python 3.12.0a5+ (heads/fix-reduce-dirty:93854e172e, Feb 10 2023, 13:41:45) [GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> x = tuple[int]
>>>
>>> it = iter(x)
>>> _ = list(it)
>>>
>>> it.__reduce__()
(<built-in function iter>, ((),))

Linked Issue

Issue: iter __reduce__ can segfault if accessing __builtins__.__dict__['iter'] mutates the iter object #101765

…e effects

…n side effects

… effects

…de effects

…o potential side effects

…r_reduce

- Added some comments for dict del usages - Switched to `__builtin__` instead of conditional `__dict__` access - Use kwargs for improved readability

…ist conditional branch

…ters

Since this could cause a segfault, it should perhaps be considered a security issue to be backported to the security branches. However, the way to trigger it is so obscure that I'm not sure it's worth backporting. @ambv what do you think?

Thanks @ionite34 for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

Sorry, @ionite34 and @JelleZijlstra, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 54dfa14c5a94b893b67a4d9e9e403ff538ce9023 3.11

Sorry @ionite34 and @JelleZijlstra, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 54dfa14c5a94b893b67a4d9e9e403ff538ce9023 3.10

@ionite34 are you interested in doing the 3.10/3.11 backports (and potentially 3.7-3.9 if we decide this is important enough)? It's probably a matter of installing cherry-picker locally and fixing a few conflicts.

@ionite34 are you interested in doing the 3.10/3.11 backports (and potentially 3.7-3.9 if we decide this is important enough)? It's probably a matter of installing cherry-picker locally and fixing a few conflicts.

Yeah I could try 👀, though uh, I don't think I saw anything on the dev guide regarding backports. Am I essentially doing the stuff mentioned here? https://pypi.org/project/cherry_picker/#example

Thanks! Yes, cherrry-picker will do most of the work of actually creating the PR, you just have to do the manual work to fix the merge.

Just as general background, we develop every 3.x version in a separate branch. PRs are usually first against the main branch (which will be 3.12). If the PR fixes a bug (as opposed to a new feature), we'll backport the change into the bugfix branches, which are currently for 3.10 and 3.11.

…`__reduce__` when internal access of `builtins.__dict__` exhausts the iterator (pythonGH-101769). (cherry picked from commit 54dfa14) Co-authored-by: Ionite <dev@ionite.io>

GH-102228 is a backport of this pull request to the 3.11 branch.

…`__reduce__` when internal access of `builtins.__dict__` exhausts the iterator (pythonGH-101769). (cherry picked from commit 54dfa14) Co-authored-by: Ionite <dev@ionite.io>

GH-102229 is a backport of this pull request to the 3.10 branch.

…uce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102228) (cherry picked from commit 54dfa14)

…uce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102229) (cherry picked from commit 54dfa14)

If it_seq is NULL then reversed is leaked here, _PyEval_GetBuiltin returns a strong reference.

Good point, but doesn't that mean the old code also leaked references? I'll prepare a PR to fix this now.

Oh wait, the N code to Py_BuildValue steals a reference, so the previous code was right in terms of refcounting.

Followup from python#101769.

Followup from #101769.

…ing (pythonGH-102265) Followup from pythonGH-101769.. (cherry picked from commit d71edbd) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

…ce__` when internal access of `builtins.__dict__` exhausts the iterator (python#101769)

Fix undefined behavior in listiter_reduce from _PyEval_GetBuiltin sid…

f256afe

…e effects

bedevere-bot added the awaiting review label Feb 10, 2023

bedevere-bot mentioned this pull request Feb 10, 2023

iter __reduce__ can segfault if accessing __builtins__.__dict__['iter'] mutates the iter object #101765

Closed

ionite34 added 4 commits February 9, 2023 20:34

Update comment to not mention __eq__

6b4faad

Fix undefined behavior in iter_reduce and calliter_reduce

a6d6211

Update listiter_reduce_general comment

4ccf427

Fix undefined behavior in bytearrayiter_reduce from _PyEval_GetBuilti…

c2c9cfb

…n side effects

arhadthedev added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Feb 10, 2023

ionite34 added 2 commits February 10, 2023 00:26

Fix undefined behavior in striter_reduce from _PyEval_GetBuiltin side…

e2989d9

… effects

Fix undefined behavior in tupleiter_reduce from _PyEval_GetBuiltin si…

71960a8

…de effects

JelleZijlstra self-requested a review February 10, 2023 05:43

JelleZijlstra reviewed Feb 10, 2023

View reviewed changes

Comment thread Objects/bytearrayobject.c Outdated

JelleZijlstra reviewed Feb 10, 2023

View reviewed changes

Comment thread Objects/listobject.c Outdated

ionite34 and others added 9 commits February 10, 2023 01:04

Move iter call in unicodeiter_reduce before it pointer access due t…

efa0540

…o potential side effects

Add iter reduce tests for issue python#101765

c5abb14

Remove C++ reference in comments

45522c6

Remove C++ reference in comments

4f5fc19

Move builtin declarations inside if blocks

049a8dd

Move _PyEval_GetBuiltin before gi checks, add gi NULL check in ga_ite…

ef4f955

…r_reduce

Update iter reduce mutating tests for generic alias

7d4afb0

📜🤖 Added by blurb_it.

8e4418d

Fix backticks format for news

d8ced8e

ionite34 marked this pull request as ready for review February 10, 2023 07:29

carljm reviewed Feb 10, 2023

View reviewed changes

Comment thread Lib/test/test_iter.py Outdated

Comment thread Lib/test/test_iter.py Outdated

Comment thread Objects/listobject.c

Comment thread Objects/unicodeobject.c

JelleZijlstra self-requested a review February 10, 2023 17:29

ionite34 added 4 commits February 10, 2023 12:46

Refactor iter reduce builtins mutation tests

178b8ea

- Added some comments for dict del usages - Switched to `__builtin__` instead of conditional `__dict__` access - Use kwargs for improved readability

Update iter mutating builtins test to include reversed iterator for l…

49ba8c3

…ist conditional branch

Add comment in unicodeiter_reduce for moving iter call before it poin…

93854e1

…ters

Change test __builtins__ to builtins import

98ec3c6

JelleZijlstra merged commit 54dfa14 into python:main Feb 24, 2023

bedevere-bot removed the awaiting merge label Feb 24, 2023

ionite34 mentioned this pull request Feb 25, 2023

[3.11] gh-101765: Fix SystemError / segmentation fault in iter __reduce__ when internal access of builtins.__dict__ exhausts the iterator (GH-101769) #102228

Merged

bedevere-bot removed the needs backport to 3.11 only security fixes label Feb 25, 2023

ionite34 mentioned this pull request Feb 25, 2023

[3.10] gh-101765: Fix SystemError / segmentation fault in iter __reduce__ when internal access of builtins.__dict__ exhausts the iterator (GH-101769) #102229

Merged

bedevere-bot removed the needs backport to 3.10 only security fixes label Feb 25, 2023

JelleZijlstra pushed a commit that referenced this pull request Feb 25, 2023

[3.11] gh-101765 : Fix SystemError / segmentation fault in iter `__red…

5d46122

…uce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102228) (cherry picked from commit 54dfa14)

JelleZijlstra pushed a commit that referenced this pull request Feb 25, 2023

[3.10] gh-101765 : Fix SystemError / segmentation fault in iter `__red…

9f472f8

…uce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102229) (cherry picked from commit 54dfa14)

kumaraditya303 reviewed Feb 25, 2023

View reviewed changes

JelleZijlstra mentioned this pull request Feb 25, 2023

gh-101765: Fix refcount issues in list and unicode pickling #102265

Merged

JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this pull request Feb 25, 2023

pythongh-101765 : Fix refcount issues in list and unicode pickling

b8ad59e

Followup from python#101769.

JelleZijlstra added a commit that referenced this pull request Feb 26, 2023

gh-101765 : Fix refcount issues in list and unicode pickling (#102265 )

d71edbd

Followup from #101769.

This was referenced Feb 26, 2023

[3.11] gh-101765: Fix refcount issues in list and unicode pickling (G… #102268

Merged

[3.10] gh-101765: Fix refcount issues in list and unicode pickling (G… #102269

Merged

corona10 mentioned this pull request Feb 8, 2024

gh-112087: Make __sizeof__ and listiter_{len, next} to be threadsafe #114843

Merged

JelleZijlstra pushed a commit to JelleZijlstra/cpython that referenced this pull request Sep 10, 2024

pythongh-101765 : Fix SystemError / segmentation fault in iter `__redu…

473a891

…ce__` when internal access of `builtins.__dict__` exhausts the iterator (python#101769)

                   } else {
+                      PyObject *reversed = _PyEval_GetBuiltin(&_Py_ID(reversed));
                       listreviterobject *it = (listreviterobject *)_it;
                       if (it->it_seq) {

gh-101765: Fix SystemError / segmentation fault in iter `__reduce__` when internal access of `builtins.__dict__` exhausts the iterator by ionite34 · Pull Request #101769 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-101765: Fix SystemError / segmentation fault in iter __reduce__ when internal access of builtins.__dict__ exhausts the iterator - #101769

gh-101765: Fix SystemError / segmentation fault in iter __reduce__ when internal access of builtins.__dict__ exhausts the iterator#101769JelleZijlstra merged 24 commits intopython:mainpython/cpython:mainfrom ionite34:fix-reduceionite34/cpython:fix-reduceCopy head branch name to clipboard

Conversation

ionite34 commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Affected methods

Linked Issue

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra commented Feb 24, 2023

Uh oh!

miss-islington commented Feb 24, 2023

Uh oh!

miss-islington commented Feb 24, 2023

Uh oh!

miss-islington commented Feb 24, 2023

Uh oh!

JelleZijlstra commented Feb 24, 2023

Uh oh!

ionite34 commented Feb 25, 2023

Uh oh!

JelleZijlstra commented Feb 25, 2023

Uh oh!

bedevere-bot commented Feb 25, 2023

Uh oh!

bedevere-bot commented Feb 25, 2023

Uh oh!

kumaraditya303 Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gh-101765: Fix SystemError / segmentation fault in iter `reduce` when internal access of `builtins.dict` exhausts the iterator by ionite34 · Pull Request #101769 · python/cpython · GitHub

gh-101765: Fix SystemError / segmentation fault in iter reduce when internal access of builtins.dict exhausts the iterator - #101769

gh-101765: Fix SystemError / segmentation fault in iter reduce when internal access of builtins.dict exhausts the iterator#101769
JelleZijlstra merged 24 commits into
python:mainfrom
ionite34:fix-reduce

ionite34 commented Feb 10, 2023 •
edited
Loading