gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it #143561

VanshAgarwal24036 · 2026-01-08T13:10:47Z

Summary

When JSONDecodeError is user-replaced and re-enters during JSON parsing,
_raise_errmsg could reuse a freed exception type, leading to a
use-after-free.

This change holds a strong reference across the call and validates the
exception type before setting it, falling back safely when needed.

Issue

Use-after-free in _json.raise_errmsg via re-entrant JSONDecodeError hook #143544

VanshAgarwal24036 · 2026-01-08T13:15:03Z

skip news

ZeroIntensity · 2026-01-08T13:21:12Z

No, this affects user-facing APIs; please add a news entry.

ZeroIntensity

Please avoid making unnecessary code changes. They make it significantly more difficult to review.
This is doing too much. The much simpler and more correct fix would be to simply move the Py_DECREF call on line 426 to after the if (exc) block.
Please add a test case and news entry.

Modules/_json.c

bedevere-app · 2026-01-08T13:30:49Z

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.

VanshAgarwal24036 · 2026-01-08T14:25:50Z

Thanks for the review.

I’ve updated the patch to keep the fix minimal, added a regression test
that restores global state, and included a NEWS entry as requested.

Lib/test/test_json/test_fail.py

Misc/NEWS.d/next/Library/2026-01-08-19-38-02.gh-issue-143544.4jRCBJ.rst

ZeroIntensity · 2026-01-08T14:30:23Z

Modules/_json.c

+
    PyObject *JSONDecodeError =
-         PyImport_ImportModuleAttr(&_Py_STR(json_decoder), &_Py_ID(JSONDecodeError));
+        PyImport_ImportModuleAttr(&_Py_STR(json_decoder), &_Py_ID(JSONDecodeError));


Again please don't make unrelated formatting changes.

Please revert this change.

ZeroIntensity · 2026-01-08T14:30:38Z

Modules/_json.c

-    if (exc) {
+    PyObject *exc = PyObject_CallFunction(JSONDecodeError, "zOn", msg, s, end);
+    if (exc != NULL) {


Same here. The only change we need is the movement of the Py_DECREF call.

Please revert this change, only move Py_DECREF(JSONDecodeError);.

Lib/test/test_json/test_fail.py

VanshAgarwal24036 · 2026-01-08T15:41:10Z

Thanks for the clarification. I’ve updated the test to only assert that the
interpreter doesn’t crash and pushed the changes.

VanshAgarwal24036 · 2026-01-08T15:43:39Z

The NEWS entry was removed per discussion above. Could someone please add the “skip news” label?

VanshAgarwal24036 · 2026-01-09T11:40:58Z

I have made the requested changes; please review again.

bedevere-app · 2026-01-09T11:41:03Z

Thanks for making the requested changes!

@ZeroIntensity: please review the changes made to this pull request.

vstinner · 2026-01-09T11:45:20Z

Modules/_json.c

+
    PyObject *JSONDecodeError =
-         PyImport_ImportModuleAttr(&_Py_STR(json_decoder), &_Py_ID(JSONDecodeError));
+        PyImport_ImportModuleAttr(&_Py_STR(json_decoder), &_Py_ID(JSONDecodeError));


Please revert this change.

vstinner · 2026-01-09T11:45:44Z

Modules/_json.c

-    if (exc) {
+    PyObject *exc = PyObject_CallFunction(JSONDecodeError, "zOn", msg, s, end);
+    if (exc != NULL) {


Please revert this change, only move Py_DECREF(JSONDecodeError);.

vstinner · 2026-01-09T11:46:01Z

Lib/test/test_json/test_fail.py


+    def test_reentrant_jsondecodeerror_does_not_crash(self):
+        # gh-143544
+        import json


Please move this import at the module top level.

Moved import json to the module top level as requested. Thanks.

vstinner · 2026-01-09T12:12:33Z

Modules/_json.c

        Py_DECREF(exc);
    }
+
+    /* Move DECREF after PyErr_SetObject */


I don't think that this comment is useful.

Removed the comment as suggested.

serhiy-storchaka

There is no good place for such test, but I think test_speedups is less wrong than test_fail.

But I suspect that it may be impossible to write a test without raising SystemError for the fixed code, which is only slightly better than a crash. If this is so, we will give up and accept a fix without unit test.

serhiy-storchaka · 2026-01-10T09:57:20Z

Lib/test/test_json/test_fail.py

+                raise self

-        hook = Trigger()
+        hook = Trigger("boom")


No, hook should be not an instance of exception, but an exception class itself.

So, you need to define __new__() to trigger execution when it is called, not __call__().

serhiy-storchaka · 2026-01-10T10:06:45Z

Lib/test/test_json/test_fail.py

+            self.assertEqual(sys.getrefcount(hook), 3)
            del hook

+            support.gc_collect()


It does not have effect here, because there are references like json.decoder.JSONDecodeError.

But calling it after del json.decoder.JSONDecodeError in __new__() will also not help, because we still have a reference as a method parameter. So, it is useless.

Maybe we could not create a good test for this issue.

VanshAgarwal24036 · 2026-01-10T11:42:43Z

Thanks for clarifying. I agree that reliably testing this without triggering a SystemError may not be feasible.
If you’re OK with proceeding without a unit test, I can drop the test changes and keep the fix only. Please let me know how you’d prefer to move forward.

serhiy-storchaka · 2026-01-10T13:25:30Z

If this is not feasible, drop the tests.

Thank you for your PR.

VanshAgarwal24036 · 2026-01-10T16:59:08Z

I’ve dropped the test as suggested since it doesn’t seem feasible to reliably reproduce this case without introducing a SystemError.
All CI checks are now passing.

Could you please re-review and let me know if anything else is needed?

vstinner

LGTM. I'm fine with no adding a test for this tricky bug.

serhiy-storchaka

LGTM.

miss-islington-app · 2026-01-12T15:39:37Z

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

bedevere-app · 2026-01-12T15:39:53Z

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

… JSONDecodeError disappears during raising it (pythonGH-143561) (cherry picked from commit c3157480601499565fd42a8afbdb0207328ac484) Co-authored-by: VanshAgarwal24036 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>

bedevere-app · 2026-01-12T15:39:59Z

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

… JSONDecodeError disappears during raising it (pythonGH-143561) (cherry picked from commit c315748) Co-authored-by: VanshAgarwal24036 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>

vstinner · 2026-01-12T15:41:44Z

Merged, thanks for the fix @VanshAgarwal24036.

…n JSONDecodeError disappears during raising it (GH-143561) (#143734) gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it (GH-143561) (cherry picked from commit c315748) Co-authored-by: VanshAgarwal24036 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>

…n JSONDecodeError disappears during raising it (GH-143561) (#143733) gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it (GH-143561) (cherry picked from commit c315748) Co-authored-by: VanshAgarwal24036 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>

VanshAgarwal24036 · 2026-01-12T16:24:53Z

Thanks a lot for the review and merge! I really appreciate it.

… JSONDecodeError disappears during raising it (python#143561) Co-authored-by: Bénédikt Tran <[email protected]>

pythongh-143544: Fix use-after-free in _json.raise_errmsg

9477824

bedevere-app bot mentioned this pull request Jan 8, 2026

Use-after-free in _json.raise_errmsg via re-entrant JSONDecodeError hook #143544

Closed

bedevere-app bot added the awaiting review label Jan 8, 2026

ZeroIntensity requested changes Jan 8, 2026

View reviewed changes

Modules/_json.c Show resolved Hide resolved

Modules/_json.c Outdated Show resolved Hide resolved

Modules/_json.c Outdated Show resolved Hide resolved

Modules/_json.c Outdated Show resolved Hide resolved

Modules/_json.c Outdated Show resolved Hide resolved

bedevere-app bot added awaiting changes and removed awaiting review labels Jan 8, 2026

VanshAgarwal24036 added 4 commits January 8, 2026 19:31

pythongh-143544: Fix use-after-free in _json.raise_errmsg

946aee3

pythongh-143544: Add regression test for re-entrant JSONDecodeError

55b4850

pythongh-143544: Add NEWS entry

1940b1e

pythongh-143544: Fix trailing whitespace in test

ed2c55c

vstinner reviewed Jan 8, 2026

View reviewed changes

Lib/test/test_json/test_fail.py Outdated Show resolved Hide resolved

Misc/NEWS.d/next/Library/2026-01-08-19-38-02.gh-issue-143544.4jRCBJ.rst Outdated Show resolved Hide resolved

ZeroIntensity reviewed Jan 8, 2026

View reviewed changes

pythongh-143544: Adjust regression test to allow SystemError

099fc4f

vstinner added the skip news label Jan 8, 2026

VanshAgarwal24036 requested a review from ZeroIntensity January 9, 2026 05:16

bedevere-app bot added awaiting change review and removed awaiting changes labels Jan 9, 2026

vstinner reviewed Jan 9, 2026

View reviewed changes

VanshAgarwal24036 added 2 commits January 9, 2026 17:30

pythongh-143544: Fix use-after-free in _json.raise_errmsg

b238345

pythongh-143544: Move json import to module level in regression test

fa69d1e

vstinner reviewed Jan 9, 2026

View reviewed changes

pythongh-143544: Remove unnecessary comment

39767c6

serhiy-storchaka reviewed Jan 10, 2026

View reviewed changes

pythongh-143544: Revert test_json regression test changes

a2e23bc

vstinner approved these changes Jan 12, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting change review labels Jan 12, 2026

serhiy-storchaka approved these changes Jan 12, 2026

View reviewed changes

serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes and removed skip news labels Jan 12, 2026

serhiy-storchaka changed the title ~~gh-143544: Fix use-after-free in _json.raise_errmsg~~ gh-143544: Fix use-after-free in the JSON decoder when JSONDecodeError disappears during raising it Jan 12, 2026

serhiy-storchaka changed the title ~~gh-143544: Fix use-after-free in the JSON decoder when JSONDecodeError disappears during raising it~~ gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it Jan 12, 2026

vstinner added the skip news label Jan 12, 2026

vstinner merged commit c315748 into python:main Jan 12, 2026
55 of 56 checks passed

bedevere-app bot removed the awaiting merge label Jan 12, 2026

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

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

reidenong pushed a commit to reidenong/cpython that referenced this pull request Jan 12, 2026

pythongh-143544: Fix possible use-after-free in the JSON decoder when…

93d59b1

… JSONDecodeError disappears during raising it (python#143561) Co-authored-by: Bénédikt Tran <[email protected]>

VanshAgarwal24036 deleted the gh-143547-unraisablehook-uaf branch January 13, 2026 05:57

Uh oh!

gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it #143561

gh-143544: Fix possible use-after-free in the JSON decoder when JSONDecodeError disappears during raising it #143561

Conversation

VanshAgarwal24036 commented Jan 8, 2026

Summary

Issue

Uh oh!

VanshAgarwal24036 commented Jan 8, 2026

Uh oh!

ZeroIntensity commented Jan 8, 2026

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app bot commented Jan 8, 2026

Uh oh!

VanshAgarwal24036 commented Jan 8, 2026

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

VanshAgarwal24036 commented Jan 8, 2026

Uh oh!

VanshAgarwal24036 commented Jan 8, 2026

Uh oh!

VanshAgarwal24036 commented Jan 9, 2026

Uh oh!

bedevere-app bot commented Jan 9, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

VanshAgarwal24036 commented Jan 10, 2026

Uh oh!

serhiy-storchaka commented Jan 10, 2026

Uh oh!

VanshAgarwal24036 commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app bot commented Jan 12, 2026

Uh oh!

VanshAgarwal24036 commented Jan 10, 2026 •

edited

Loading