gh-145678: Fix use-after-free in itertools.groupby _grouper_next() by sampsonc · Pull Request #145679 · python/cpython

sampsonc · 2026-03-09T14:42:05Z

Summary

Fixes a use-after-free (UAF) in _grouper_next() in Modules/itertoolsmodule.c.

Root Cause

_grouper_next() passed igo->tgtkey and gbo->currkey directly to
PyObject_RichCompareBool() without first holding strong references.
A user-defined __eq__ can re-enter the parent groupby iterator during
the comparison. That re-entry calls groupby_step(), which executes:

Py_XSETREF(gbo->currkey, newkey);

This frees gbo->currkey while it is still under comparison — a use-after-free.

Fix

Take INCREF'd local snapshots before calling PyObject_RichCompareBool(),
mirroring the protection added to groupby_next() in gh-143543:

PyObject *tgtkey = igo->tgtkey;
PyObject *currkey = gbo->currkey;
Py_INCREF(tgtkey);
Py_INCREF(currkey);
rcmp = PyObject_RichCompareBool(tgtkey, currkey, Py_EQ);
Py_DECREF(tgtkey);
Py_DECREF(currkey);

Test plan

./python -m test test_itertools -k test_grouper_next_reentrant_eq_does_not_crash
Run with AddressSanitizer (./configure --with-pydebug && make with ASAN enabled) to confirm no UAF

Closes gh-145678.

🤖 Generated with Claude Code

python-cla-bot · 2026-03-09T14:42:10Z

All commit authors signed the Contributor License Agreement.

_grouper_next() passed igo->tgtkey and gbo->currkey directly to PyObject_RichCompareBool() without first holding strong references. A re-entrant __eq__ that advanced the parent groupby iterator would call groupby_step(), which executes Py_XSETREF(gbo->currkey, newkey), freeing currkey while it was still under comparison. Fix by taking INCREF'd local snapshots before the comparison, mirroring the protection added to groupby_next() in pythongh-143543. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sampsonc · 2026-03-09T15:24:09Z

The two CI failures (Windows free-threading arm64 and Docs) are pre-existing issues in main unrelated to this PR. The docs check-warnings.py script confirmed zero new warnings from our changes, and the Windows failure is ENV_CHANGED from test_multiprocessing_spawn.test_threads.

encukou · 2026-03-09T15:50:12Z

Modules/itertoolsmodule.c

+    /* A user-defined __eq__ can re-enter groupby (via the parent iterator)
+       and call groupby_step(), which frees gbo->currkey via Py_XSETREF while
+       we are still comparing it.  Take local snapshots with strong references
+       so INCREF/DECREF apply to the same objects even under re-entrancy. */
+    PyObject *tgtkey = igo->tgtkey;
+    PyObject *currkey = gbo->currkey;
+    Py_INCREF(tgtkey);
+    Py_INCREF(currkey);


I don't think we need to record historical issues in the source. General reasons for holding refs while calling user code are clear -- the trick is finding cases where this was forgotten.

Suggested change

/* A user-defined __eq__ can re-enter groupby (via the parent iterator)

and call groupby_step(), which frees gbo->currkey via Py_XSETREF while

we are still comparing it. Take local snapshots with strong references

so INCREF/DECREF apply to the same objects even under re-entrancy. */

PyObject *tgtkey = igo->tgtkey;

PyObject *currkey = gbo->currkey;

Py_INCREF(tgtkey);

Py_INCREF(currkey);

PyObject *tgtkey = Py_NewRef(igo->tgtkey);

PyObject *currkey = Py_NewRef(gbo->currkey);

(cc @picnixz, reviewer of the earlier PR)

I don't think we need to record historical issues in the source

Victor sometimes asks me to do that, while Serhiy usually prefers not to, and I personally avoid but I did so in the past so I'm not that consistent either. I think I did add one such comment in OrderedDict so I don't mind.

+1 for Py_NewRef, it slipped through my review my bad.

IMO, comments are good when it's not clear why the code does something. Here, it's relatively clear that we're calling user code. The exact way user code can clear references isn't that important -- and may change in the future.

Py_NewRef: no worries, that's a style nitpick.

encukou · 2026-03-09T15:57:48Z

Lib/test/test_itertools.py

+        values = [1, 1, 2]
+        keys_iter = iter([Key(1, True), Key(1, False), Key(2, False)])
+        g = itertools.groupby(values, lambda _: next(keys_iter))
+        outer_grouper = g


Just call it outer_grouper, or g. No need for two names.

encukou · 2026-03-09T16:00:06Z

Lib/test/test_itertools.py

+        # regression test for gh-145678: _grouper_next() did not protect
+        # gbo->currkey / igo->tgtkey before calling PyObject_RichCompareBool,
+        # so a reentrant __eq__ that advanced the parent groupby could free
+        # those objects while they were still being compared (use-after-free).


The reference is enough.

Suggested change

# regression test for gh-145678: _grouper_next() did not protect

# gbo->currkey / igo->tgtkey before calling PyObject_RichCompareBool,

# so a reentrant __eq__ that advanced the parent groupby could free

# those objects while they were still being compared (use-after-free).

# regression test for gh-145678

encukou · 2026-03-09T16:00:15Z

Lib/test/test_itertools.py

+                    # Advance the parent groupby iterator from inside __eq__,
+                    # which calls groupby_step() and frees the old currkey.
+                    try:
+                        next(outer_grouper)
+                    except StopIteration:
+                        pass


The try/except is not necessary; we don't get here with an exhausted iterator.

Lose the comment.

encukou · 2026-03-09T16:00:28Z

Misc/NEWS.d/next/Library/2026-03-09-00-00-00.gh-issue-145678.grouper-uaf.rst

+Fix a use-after-free in :func:`itertools.groupby`: the internal
+``_grouper_next()`` function did not hold strong references to
+``gbo->currkey`` and ``igo->tgtkey`` before calling
+:c:func:`PyObject_RichCompareBool`, so a re-entrant ``__eq__`` that advanced
+the parent iterator (triggering ``groupby_step()`` and ``Py_XSETREF`` on
+``currkey``) could free those objects while they were still being compared.
+The fix mirrors the protection added in :gh:`143543` for ``groupby_next()``.


Suggested change

Fix a use-after-free in :func:`itertools.groupby`: the internal

``_grouper_next()`` function did not hold strong references to

``gbo->currkey`` and ``igo->tgtkey`` before calling

:c:func:`PyObject_RichCompareBool`, so a re-entrant ``__eq__`` that advanced

the parent iterator (triggering ``groupby_step()`` and ``Py_XSETREF`` on

``currkey``) could free those objects while they were still being compared.

The fix mirrors the protection added in :gh:`143543` for ``groupby_next()``.

Fix a use-after-free crash in :func:`itertools.groupby` when a user-defined

``__eq__`` advanced the parent iterator while the iterator of groups was advanced.

The fix mirrors the protection added in :gh:`143543` for ``groupby_next()``.

sampsonc requested a review from rhettinger as a code owner March 9, 2026 14:42

bedevere-app bot mentioned this pull request Mar 9, 2026

itertools.groupby: use-after-free in _grouper_next() when __eq__ re-enters the iterator #145678

Open

bedevere-app bot added the awaiting review label Mar 9, 2026

sampsonc force-pushed the gh-145678-grouper-uaf branch from eb0bb61 to 79bcea0 Compare March 9, 2026 15:12

encukou reviewed Mar 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

gh-145678: Fix use-after-free in itertools.groupby _grouper_next()#145679

gh-145678: Fix use-after-free in itertools.groupby _grouper_next()#145679
sampsonc wants to merge 1 commit intopython:mainfrom
sampsonc:gh-145678-grouper-uaf

sampsonc commented Mar 9, 2026

Uh oh!

python-cla-bot bot commented Mar 9, 2026 •

edited

Loading

Uh oh!

sampsonc commented Mar 9, 2026

Uh oh!

encukou Mar 9, 2026

Uh oh!

picnixz Mar 9, 2026

Uh oh!

encukou Mar 10, 2026

Uh oh!

encukou Mar 9, 2026

Uh oh!

encukou Mar 9, 2026

Uh oh!

encukou Mar 9, 2026

Uh oh!

encukou Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sampsonc commented Mar 9, 2026

Summary

Root Cause

Fix

Test plan

Uh oh!

python-cla-bot bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sampsonc commented Mar 9, 2026

Uh oh!

encukou Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

picnixz Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

encukou Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

encukou Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

encukou Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

encukou Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

encukou Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

python-cla-bot bot commented Mar 9, 2026 •

edited

Loading