Skip to content

gh-146613: Fix re-entrant use-after-free in itertools._grouper#147962

Open
TheSkyC wants to merge 8 commits intopython:mainfrom
TheSkyC:gh-146613-itertools-uaf
Open

gh-146613: Fix re-entrant use-after-free in itertools._grouper#147962
TheSkyC wants to merge 8 commits intopython:mainfrom
TheSkyC:gh-146613-itertools-uaf

Conversation

@TheSkyC
Copy link
Copy Markdown

@TheSkyC TheSkyC commented Apr 1, 2026 •
edited
Loading

Closes gh-146613

The same pattern was fixed in groupby.__next__ (gh-143543 / a91b5c3), but _grouper_next (the inner group iterator returned by groupby) was missed.

A user-defined __eq__ can re-enter the grouper during PyObject_RichCompareBool, causing Py_XSETREF to free currkey while it is still being used.

Fixed by taking strong references (Py_INCREF / Py_DECREF) to tgtkey and currkey before the comparison, exactly as done in groupby_next.

Added regression test test_grouper_reentrant_eq_does_not_crash.

@TheSkyC
pythongh-146613: Fix re-entrant use-after-free in itertools._grouper
9aa672e 
The same pattern was fixed in groupby.__next__ (pythongh-143543 / a91b5c3),
but _grouper_next (the inner group iterator returned by groupby)
was missed.

A user-defined __eq__ can re-enter the grouper during
PyObject_RichCompareBool, causing Py_XSETREF to free currkey
while it is still being used.

Fix by taking local snapshots of tgtkey/currkey + INCREF/DECREF
protection, exactly as done in groupby_next.

Added regression test in test_itertools.py.
@TheSkyC TheSkyC requested a review from rhettinger as a code owner April 1, 2026 11:03
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 1, 2026

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.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Apr 1, 2026 •
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app bot mentioned this pull request Apr 1, 2026
Open
vstinner
vstinner reviewed Apr 1, 2026
View reviewed changes
@TheSkyC
Copy link
Copy Markdown
Author

TheSkyC commented Apr 1, 2026

Hi @vstinner,
I've addressed your review comments in the latest commit
Thanks for your guidance!

vstinner
vstinner approved these changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
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.

I confirm that the test does crash without the fix, and does pass successfully with the fix.

@TheSkyC @vstinner
Update Misc/NEWS.d/next/Library/2026-04-01-11-05-36.gh-issue-146613.G…
895b685 
…zjUFK.rst

Co-authored-by: Victor Stinner <vstinner@python.org>
@vstinner vstinner enabled auto-merge (squash) April 1, 2026 13:00
@vstinner vstinner added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Apr 1, 2026
picnixz
picnixz reviewed Apr 1, 2026
View reviewed changes
@TheSkyC @picnixz
Update Modules/itertoolsmodule.c
fef6412 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
auto-merge was automatically disabled April 1, 2026 13:16

Head branch was pushed to by a user without write access

@TheSkyC @picnixz
Update Misc/NEWS.d/next/Library/2026-04-01-11-05-36.gh-issue-146613.G…
fd9b831 
…zjUFK.rst

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@vstinner vstinner mentioned this pull request Apr 1, 2026
Merged
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

Lint CI job failed with:

  error[: unpinned action reference
    --> .github/workflows/reusable-check-html-ids.yml:19:15
     |
  19 |         uses: actions/checkout@v6
     |               ^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)

It's unrelated to this change. I reported the issue to: #145632 (comment).

@TheSkyC
Copy link
Copy Markdown
Author

TheSkyC commented Apr 1, 2026

Thanks for reporting the issue, @vstinner! Should I update the branch from main to fix it?

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

Thanks for reporting the issue, @vstinner! Should I update the branch from main to fix it?

The issue is not fixed yet, so there is no need to update your branch.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

#147968 will fix the Lint CI.

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

Reviewers

@picnixz picnixz picnixz left review comments

@vstinner vstinner vstinner approved these changes

@rhettinger rhettinger Awaiting requested review from rhettinger rhettinger is a code owner

Assignees

No one assigned

Labels

awaiting merge needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

itertools._grouper.__next__ has the same re-entrant use-after-free that was fixed in groupby.__next__

3 participants

@TheSkyC @vstinner @picnixz