gh-142831: Fix UAF in _json module#142851
Conversation
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Could you add tests?
kumaraditya303
added
needs backport to 3.13
|
Added tests |
|
I pushed a minor test change to make the test run on both pure python and C encoder. |
|
@kumaraditya303 But the new tests don't reproduce the issue! |
This reverts commit 66c3af1.
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. |
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.
|
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. |
- 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.
|
Sorry, @ashm-dev and @gpshead, I could not cleanly backport this to |
|
Sorry, @ashm-dev and @gpshead, I could not cleanly backport this to |
… mutation (pythongh-142851) Hold strong references to borrowed items unconditionally (not only in free-threading builds) in _encoder_iterate_mapping_lock_held and _encoder_iterate_fast_seq_lock_held. User callbacks invoked during encoding can mutate or clear the underlying container, invalidating borrowed references. The dict iteration path was already fixed by pythongh-145244. Co-authored-by: Kumar Aditya <kumaraditya@python.org> Co-authored-by: Gregory P. Smith <greg@krypto.org>
|
@ashm-dev are you able to do the backports (see the bot's comments above for instructions)? |
|
@StanFromIreland Yes, of course I will |
|
GH-150078 is a backport of this pull request to the 3.14 branch. |
|
GH-150079 is a backport of this pull request to the 3.13 branch. |
|
@StanFromIreland backports done — manually because the bot couldn't cherry-pick cleanly (the iteration loops in 3.13/3.14 had not yet been extracted into the
Both built locally with |
…t mutation (gh-142851) (#150078) gh-142831: Fix use-after-free in json encoder during re-entrant mutation (gh-142851) User callbacks invoked during JSON encoding (e.g. the `default` callback or a custom string encoder) can mutate or clear the dict or sequence being encoded, invalidating borrowed references to items, keys, and values. Hold strong references unconditionally while iterating. (cherry picked from commit 235fa72) Co-authored-by: Kumar Aditya <kumaraditya@python.org> Co-authored-by: Gregory P. Smith <greg@krypto.org>
…t mutation (gh-142851) (#150079) gh-142831: Fix use-after-free in json encoder during re-entrant mutation (gh-142851) User callbacks invoked during JSON encoding (e.g. the `default` callback or a custom string encoder) can mutate or clear the dict or sequence being encoded, invalidating borrowed references to items, keys, and values. Hold strong references unconditionally while iterating. (cherry picked from commit 235fa72) Co-authored-by: Kumar Aditya <kumaraditya@python.org> Co-authored-by: Gregory P. Smith <greg@krypto.org>
|
thanks! |
8 participants
json.encodermapping iteration via re-entrant key encoder #142831Fixes #142831