Skip to content

gh-142831: Fix UAF in _json module#142851

Merged
gpshead merged 25 commits intopython:mainfrom
ashm-dev:json
Apr 12, 2026
Merged

gh-142831: Fix UAF in _json module#142851
gpshead merged 25 commits intopython:mainfrom
ashm-dev:json

Conversation

@ashm-dev
Copy link
Copy Markdown
Contributor

@ashm-dev ashm-dev commented Dec 17, 2025

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add tests?

@kumaraditya303 kumaraditya303 added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Dec 17, 2025
@ashm-dev
Copy link
Copy Markdown
Contributor Author

Added tests

@kumaraditya303
Copy link
Copy Markdown
Contributor

I pushed a minor test change to make the test run on both pure python and C encoder.

@ashm-dev
Copy link
Copy Markdown
Contributor Author

@kumaraditya303 But the new tests don't reproduce the issue!
P.S. Here is how I verify the tests: I build Python from main with ASan, and if the tests trigger the leak (the one reported in the issue), then they are good; otherwise, they are not.

This reverts commit 66c3af1.
@kumaraditya303
Copy link
Copy Markdown
Contributor

But the new tests don't reproduce the issue!

I was able to reproduce it but now I can't, not sure if it was a build cache issue or something. Sorry reverted it back.

serhiy-storchaka

This comment was marked as resolved.

@ashm-dev
Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 11, 2026

Thanks for making the requested changes!

@serhiy-storchaka, @kumaraditya303: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from serhiy-storchaka March 11, 2026 16:31
ashm-dev and others added 6 commits March 11, 2026 19:37
_json.c is a library module; the identical fix pythongh-145244 used Library.
Match the style of the comment added by pythongh-145244 for the dict
iteration case.  These explain why borrowed references from items
lists and sequences must be held as strong references during encoding.
@gpshead gpshead self-assigned this Apr 11, 2026
@gpshead
Copy link
Copy Markdown
Member

gpshead commented Apr 11, 2026

nothing the irony of my commit re-adding explanatory comments that earlier reviewer wanted removed. the other PR I merged intersecting with this region and bug had such a comment so put them in here as well. i'll re-look at the whole and pick a direction in a bit.

gpshead added 2 commits April 11, 2026 23:33
- Replace bare try/except with direct calls; both tests should
  succeed without raising since encode_str and default return valid
  serializable values.
- Assert that the mutation path was actually exercised (cleared flag,
  call_count).
- Trigger list mutation on 3rd element instead of 1st, so the loop
  processes multiple items before the list is cleared mid-iteration.
- Add comments explaining what each test verifies.
@gpshead gpshead enabled auto-merge (squash) April 11, 2026 23:54
@gpshead gpshead merged commit 235fa72 into python:main Apr 12, 2026
62 of 63 checks passed
@miss-islington-app
Copy link
Copy Markdown

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

@miss-islington-app
Copy link
Copy Markdown

Sorry, @ashm-dev and @gpshead, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 235fa7244a0474c492ae98ee444529c7ba2a9047 3.14

@miss-islington-app
Copy link
Copy Markdown

Sorry, @ashm-dev and @gpshead, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 235fa7244a0474c492ae98ee444529c7ba2a9047 3.13

@ashm-dev ashm-dev deleted the json branch April 12, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use-after-free in json.encoder mapping iteration via re-entrant key encoder

6 participants