gh-125969: fix OOB in `future_schedule_callbacks` due to an evil `call_soon` by picnixz · Pull Request #125970 · python/cpython

picnixz · 2024-10-25T09:19:23Z

Issue: Evil call_soon may cause OOB in future_schedule_callbacks #125969

asvetlov

Good catch!

Please address notes.

bedevere-app · 2024-10-25T12:08:47Z

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.

picnixz · 2024-10-25T12:33:39Z

I have made the requested changes; please review again.

bedevere-app · 2024-10-25T12:33:44Z

Thanks for making the requested changes!

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

asvetlov · 2024-10-25T14:24:53Z

LGTM!

Nico-Posada · 2024-10-25T14:57:43Z

This might need a new issue opened, but you can still UAF on callback0 using a malicious getattribute in the loop obj

importasyncioclassEvilLoop:
defcall_soon(*args):
# will crash before it actually gets here print(args)
defget_debug(self):
returnFalsedef__getattribute__(self, name):
globaltrackerifname=="call_soon":
fut.remove_done_callback(tracker)
deltrackerprint("returning get_soon fn after clearing callback0")
returnobject.__getattribute__(self, name)
classTrackDel:
def__del__(self):
print("deleted", self)
fut=asyncio.Future(loop=EvilLoop())
tracker=TrackDel()
fut.add_done_callback(tracker)
fut.set_result("kaboom")

picnixz · 2024-10-25T15:03:06Z

This might need a new issue opened, but you can still UAF on callback0 using a malicious getattribute in the loop obj

Let's have another issue for this one. I only noticed the OOB issue when splitting the PR.

See: #125984.

willingc · 2024-10-25T16:22:21Z

Thanks @asvetlov for the review. I hope all is going well with you.

asvetlov · 2024-10-25T17:09:15Z

Thank you, @willingc, I'm good.
Yeah, last years were hard to me.

kumaraditya303 · 2024-10-25T18:11:29Z

Good to see you back @asvetlov

miss-islington-app · 2024-10-25T18:15:19Z

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

…l `call_soon` (pythonGH-125970) (cherry picked from commit c5b99f5) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

bedevere-app · 2024-10-25T18:15:40Z

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

bedevere-app · 2024-10-25T18:15:45Z

GH-125992 is a backport of this pull request to the 3.12 branch.

…il `call_soon` (GH-125970) (#125992) gh-125969: fix OOB in `future_schedule_callbacks` due to an evil `call_soon` (GH-125970) (cherry picked from commit c5b99f5) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

…il `call_soon` (GH-125970) (#125991) gh-125969: fix OOB in `future_schedule_callbacks` due to an evil `call_soon` (GH-125970) (cherry picked from commit c5b99f5) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

asvetlov · 2024-10-25T19:18:55Z

thanks, @kumaraditya303

…l `call_soon` (python#125970) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

picnixz added 2 commits October 25, 2024 11:18

fix OOB in future_schedule_callbacks
9f64043

blurb
aa62841

picnixz requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners October 25, 2024 09:19

bedevere-appBot mentioned this pull request Oct 25, 2024
Evil call_soon may cause OOB in future_schedule_callbacks#125969
Closed

bedevere-appBot added the awaiting review label Oct 25, 2024

picnixz added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 25, 2024

graingert reviewed Oct 25, 2024
View reviewed changes

Comment threadModules/_asynciomodule.c Outdated

fixup rst?
d210162

picnixz mentioned this pull request Oct 25, 2024
gh-125789: fix side-effects in asyncio callback scheduling methods #125833
Closed

fixup
0dec70d

graingert reviewed Oct 25, 2024
View reviewed changes

Comment threadMisc/NEWS.d/next/Library/2024-10-25-11-13-24.gh-issue-125969.YvbrTr.rst

asvetlov requested changes Oct 25, 2024
View reviewed changes

Comment threadModules/_asynciomodule.c Outdated
Comment threadModules/_asynciomodule.c Outdated

bedevere-appBot added awaiting changes and removed awaiting review labels Oct 25, 2024

asvetlov self-assigned this Oct 25, 2024

picnixz added 2 commits October 25, 2024 14:11

revert cosmetic change
db96052

Copy the callback list before consuming callbacks.
f332de9

bedevere-appBot added awaiting change review and removed awaiting changes labels Oct 25, 2024

bedevere-appBot requested a review from asvetlov October 25, 2024 12:33

kumaraditya303 added the topic-asyncio label Oct 25, 2024

asvetlov approved these changes Oct 25, 2024
View reviewed changes

bedevere-appBot added awaiting merge and removed awaiting change review labels Oct 25, 2024

Merge branch 'main' into fix/future-segfault-125969
82069f3

kumaraditya303 reviewed Oct 25, 2024
View reviewed changes

Comment threadModules/_asynciomodule.c Outdated

Transfer ownership instead of incref/decref
4197ed4

picnixz mentioned this pull request Oct 25, 2024
UAF on fut->fut_{callback,context}0 with evil __getattribute__ in _asynciomodule.c#125984
Closed

picnixz requested review from asvetlov and kumaraditya303 October 25, 2024 15:26

asvetlov approved these changes Oct 25, 2024
View reviewed changes

kumaraditya303 approved these changes Oct 25, 2024
View reviewed changes

kumaraditya303 merged commit c5b99f5 into python:mainOct 25, 2024

bedevere-appBot removed the awaiting merge label Oct 25, 2024

bedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Oct 25, 2024

bedevere-appBot removed the needs backport to 3.12 only security fixes label Oct 25, 2024

picnixz deleted the fix/future-segfault-125969 branch October 25, 2024 18:20

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
pythongh-125969: fix OOB infuture_schedule_callbacksdue to an evi…
44bf6d6
…l `call_soon` (python#125970) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

Uh oh!

Uh oh!

Conversation

picnixz commented Oct 25, 2024• edited by bedevere-app BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bedevere-appBot commented Oct 25, 2024

Uh oh!

picnixz commented Oct 25, 2024

Uh oh!

bedevere-appBot commented Oct 25, 2024

Uh oh!

asvetlov commented Oct 25, 2024

Uh oh!

Uh oh!

Nico-Posada commented Oct 25, 2024

Uh oh!

picnixz commented Oct 25, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willingc commented Oct 25, 2024

Uh oh!

asvetlov commented Oct 25, 2024

Uh oh!

kumaraditya303 commented Oct 25, 2024

Uh oh!

miss-islington-appBot commented Oct 25, 2024

Uh oh!

bedevere-appBot commented Oct 25, 2024

Uh oh!

bedevere-appBot commented Oct 25, 2024

Uh oh!

asvetlov commented Oct 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

picnixz commented Oct 25, 2024•
edited by bedevere-app Bot
Loading

picnixz commented Oct 25, 2024•
edited
Loading