gh-142663: Fix use-after-free in memoryview comparison by superboy-zjc · Pull Request #143305 · python/cpython

superboy-zjc · 2025-12-31T06:53:28Z

When comparing two memoryview objects with different formats, memory_richcompare uses the struct module to unpack elements. A custom struct.Struct.unpack_from implementation could releases and resizes underlying buffer, which invalidates the buffer pointer, during iteration. This leads to a use-after-free when the comparison loop continued accessing the freed memory.

The fix increments the exports count of the memoryview objects before performing the comparison, effectively locking the buffers. This mirrors the protection already provided for non-memoryview objects via PyObject_GetBuffer.

Issue: Use-after-free in memoryview comparison via re-entrant struct.Struct.unpack_from #142663

When comparing two memoryview objects with different formats, `memory_richcompare` uses the `struct` module to unpack elements. A custom `struct.Struct.unpack_from` implementation could releases and resizes underlying buffer, which invalidates the buffer pointer, during iteration. This leads to a use-after-free when the comparison loop continued accessing the freed memory. The fix increments the `exports` count of the memoryview objects before performing the comparison, effectively locking the buffers. This mirrors the protection already provided for non-memoryview objects via `PyObject_GetBuffer`.

…he type check result

…42663

superboy-zjc · 2026-01-03T20:12:12Z

All resolved. Thanks for the review!

sobolevn · 2026-01-03T22:04:07Z

+                mv1.release()
+                src1.append(3.14)
+                return (1,)
+        with support.swap_attr(struct, "Struct", S):


This line puzzles me a little bit. When you swap classes in the stdlib, you can get unexpected results. Can we replicate this without monkeypatching?

Hi @sobolevn I doubt the possibility of replicating this without monkeypatching. The issue stems from, during a mixed format buffer comparison, memory_richcompare function calls a re-entrant struct.Struct.unpack_from that is loaded by a PyImport_ImportModuleAttrString("struct", "Struct") call in the struct_get_unpacker. Is there anyway you would recommend to work around it ?

/* Return a new unpacker for the given format. */ static struct unpacker * struct_get_unpacker(const char *fmt, Py_ssize_t itemsize) { ... // struct.Struct Importation Struct = PyImport_ImportModuleAttrString("struct", "Struct"); if (Struct == NULL) return NULL; x = unpacker_new(); ... x->unpack_from = PyObject_GetAttrString(structobj, "unpack_from"); if (x->unpack_from == NULL) goto error; ... }

There are countless potential issues if we replace stdlib types with something else. Let's please first discuss in the issue whether this is even a valid bug report.

I did consider it as a crash because we also fixed similar occurrences elsewhere. If we just had an exception it would have been ok, but here we have a crash. We do not want this.

I see there are many tests having monkey patch. Why does this case make difference? Is there any update regarding this pr or should we @ more core dev to take a look?

struct_get_unpacker looks inefficient. We may add a cache for struct.Struct in future. Or we can cache the bound unpack_from() methods. Anyway, monkey-patching struct.Struct will either not have any effect or will affect all following code. I suspect that after I wrote this, someone might take it as a challenge.

I do not see a good solution for this. We might run the tests in a subprocess to reduce affect on other code, but this is too much hassle (and this is not 100% future proof). Perhaps it would be safer to not add a test.

Drop the tests, future changes can make it unsafe. Add a simple reproducer script in the PR or in the issue, so we can test the fix manually.

github-actions · 2026-05-04T07:01:36Z

This PR is stale because it has been open for 30 days with no activity.

serhiy-storchaka

I updated the branch and resolved conflict with main. I left few review comments. After addressing them, I will approve and merge this PR. Thank you for your contribution.

serhiy-storchaka · 2026-07-13T05:39:12Z

+    /* Prevent memoryview object from being released and its underlying buffer
+       reshaped during a mixed format comparison loop. */
+    // See https://github.com/python/cpython/issues/142663.
+    ((PyMemoryViewObject *)v)->exports++;


All other modifications of exports use FT_ATOMIC_ADD_SSIZE. Please use it here and in other 3 sites.

Suggested change

((PyMemoryViewObject *)v)->exports++;

FT_ATOMIC_ADD_SSIZE(((PyMemoryViewObject *)v)->exports, 1);

serhiy-storchaka · 2026-07-13T05:41:29Z

+    /* Prevent memoryview object from being released and its underlying buffer
+       reshaped during a mixed format comparison loop. */
+    // See https://github.com/python/cpython/issues/142663.


Do not mix different types of comments, especially in such proximity. Drop the URL, the NEWS/PR already carry the issue link.

serhiy-storchaka · 2026-07-13T05:48:01Z

+                mv1.release()
+                src1.append(3.14)
+                return (1,)
+        with support.swap_attr(struct, "Struct", S):


Drop the tests, future changes can make it unsafe. Add a simple reproducer script in the PR or in the issue, so we can test the fix manually.

bedevere-app · 2026-07-13T05:52:08Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

vstinner · 2026-07-13T05:55:47Z

I'm not convinced that we have to fix this issue. Sure, if you do silly things like replacing a stdlib method which do weird things, the stdlib can misbehave including crashing.

If I understood, the reproducer comes from fuzzing Python. I don't think that a "regular Python code" using struct which doesn't monkey-patch the struct module can crash Python.

If someone can show me a code to trigger the bug without replacing struct methods, I would be interested. Otherwise, I suggest closing the issue as "wont't fix".

superboy-zjc · 2026-07-13T06:07:22Z

I'm not convinced that we have to fix this issue. Sure, if you do silly things like replacing a stdlib method which do weird things, the stdlib can misbehave including crashing.

If I understood, the reproducer comes from fuzzing Python. I don't think that a "regular Python code" using struct which doesn't monkey-patch the struct module can crash Python.

If someone can show me a code to trigger the bug without replacing struct methods, I would be interested. Otherwise, I suggest closing the issue as "wont't fix".

@vstinner, regular Python code surely normally doesn’t look like this. But there are usage scenarios where untrusted users are allowed to run arbitrary Python code, such as Python-based sandboxes like RestrictedPython. In these environments, a memory corruption bug can potentially be exploited to achieve remote code execution, which is exactly what we don’t want and why I think it should be fixed.

serhiy-storchaka · 2026-07-13T06:50:40Z

Replacing a stdlib method which do weird things is just a way to get a stable reproducer for rare race condition. The bug exists, even if it is difficult to reproduce it other way. The fix is simple, we should not hold it. The tests were nice, but they have potential to poison interpreter after future changes in struct, so it is is safer to drop them. They are not worth running a subprocess.

vstinner · 2026-07-13T06:52:56Z

There are plently of ways of crashing Python with "pure Python code". Just an example: import ctypes; ctypes.string_at(0). It's not possible to write a safe sandbox in CPython. I know it well, I tried to write my own sandbox: https://lwn.net/Articles/574215/

It doesn't mean that we have every ways to crash Python. You're not suppose to override struct method to release and modify a memoryview.

serhiy-storchaka · 2026-07-13T07:29:24Z

I do not even consider sandbox as a use case. But we have potential race condition. Other code in this file is already guarder, as well as similar code in other file. We should do this just for consistency.

bedevere-app Bot added the awaiting review label Dec 31, 2025

bedevere-app Bot mentioned this pull request Dec 31, 2025

Use-after-free in memoryview comparison via re-entrant struct.Struct.unpack_from #142663

Open

picnixz reviewed Dec 31, 2025

View reviewed changes

Comment thread Objects/memoryobject.c Outdated

picnixz reviewed Dec 31, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst Outdated

picnixz reviewed Dec 31, 2025

View reviewed changes

Comment thread Lib/test/test_memoryview.py Outdated

Comment thread Lib/test/test_memoryview.py Outdated

superboy-zjc added 2 commits January 3, 2026 12:01

pythongh-142663: resolving comments by refactoring unit tests and cac…

06846a9

…he type check result

Merge branch 'main' of https://github.com/python/cpython into issue-1…

0694b20

…42663

picnixz self-requested a review January 3, 2026 20:43

sobolevn reviewed Jan 3, 2026

View reviewed changes

serhiy-storchaka self-requested a review January 12, 2026 18:48

github-actions Bot added the stale Stale PR or inactive for long period of time. label May 4, 2026

Merge branch 'main' into issue-142663

8a90934

serhiy-storchaka requested changes Jul 13, 2026

View reviewed changes

bedevere-app Bot added awaiting changes and removed awaiting review labels Jul 13, 2026

	((PyMemoryViewObject *)v)->exports++;
	FT_ATOMIC_ADD_SSIZE(((PyMemoryViewObject *)v)->exports, 1);

Uh oh!

Uh oh!

Conversation

superboy-zjc commented Dec 31, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

superboy-zjc commented Jan 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

superboy-zjc Jan 6, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Jul 13, 2026

Uh oh!

vstinner commented Jul 13, 2026

Uh oh!

superboy-zjc commented Jul 13, 2026

Uh oh!

serhiy-storchaka commented Jul 13, 2026

Uh oh!

vstinner commented Jul 13, 2026

Uh oh!

serhiy-storchaka commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

superboy-zjc commented Dec 31, 2025 •

edited by bedevere-app Bot

Loading

superboy-zjc Jan 6, 2026 •

edited

Loading