Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Out of bounds write in _asyncio_Future_remove_done_callback #75011

Closed
NedWilliamson mannequin opened this issue Jul 2, 2017 · 3 comments
Closed

Out of bounds write in _asyncio_Future_remove_done_callback #75011

NedWilliamson mannequin opened this issue Jul 2, 2017 · 3 comments
Labels
3.7 (EOL) end of life topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@NedWilliamson
Copy link
Mannequin

NedWilliamson mannequin commented Jul 2, 2017

BPO 30828
Nosy @1st1
PRs
  • bpo-30828: Fix OOB write in asyncio.CFuture.remove_done_callback #2569
  • [3.6] bpo-30828: Fix out of bounds write in `asyncio.CFuture.remove_d… #2590
    		•
    Files
  • fix.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2017-09-03.05:59:17.245>
created_at = <Date 2017-07-02.12:24:36.955>
labels = ['3.7', 'type-crash', 'expert-asyncio']
title = 'Out of bounds write in _asyncio_Future_remove_done_callback'
updated_at = <Date 2017-09-03.05:59:17.244>
user = 'https://bugs.python.org/NedWilliamson'

    bugs.python.org fields:

    activity = <Date 2017-09-03.05:59:17.244>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-03.05:59:17.245>
closer = 'serhiy.storchaka'
components = ['asyncio']
creation = <Date 2017-07-02.12:24:36.955>
creator = 'Ned Williamson'
dependencies = []
files = ['46987']
hgrepos = []
issue_num = 30828
keywords = ['patch']
message_count = 3.0
messages = ['297513', '297774', '297775']
nosy_count = 2.0
nosy_names = ['yselivanov', 'Ned Williamson']
pr_nums = ['2569', '2590']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue30828'
versions = ['Python 3.7']
    @NedWilliamson
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Jul 2, 2017

    This is very similar to the issue reported in https://bugs.python.org/issue28963 - this function is still buggy when items are pushed onto the done callbacks, as the new list is assumed to be large enough.

    The issue was pointed out on the code review page here: http://www.psf.upfronthosting.co.za/review/28963/ but it seems it was missed.

    import asyncio

fut = asyncio.Future()
fut.add_done_callback(str)

for _ in range(63):
    fut.add_done_callback(id)

class evil:
    def __eq__(self, other):
        fut.add_done_callback(id)
        return False

fut.remove_done_callback(evil())

    This testcase leads to a crash due to the out of bounds access:

    ASAN:DEADLYSIGNAL
=================================================================
==21457==ERROR: AddressSanitizer: SEGV on unknown address 0x00010f99fc90 (pc 0x00010f99fc90 bp 0x7fff53c18f70 sp 0x7fff53c18eb8 T0)
    #0 0x10f99fc8f  (<unknown module>)
    #1 0x10c2675cb in _PyEval_EvalFrameDefault ceval.c:2330
    #2 0x10c03f19f in function_code_fastcall call.c:282
    #3 0x10c03cf2f in _PyFunction_FastCallDict call.c:328
    #4 0x10c040acf in _PyObject_FastCall_Prepend call.c:844
    #5 0x10c12de9c in slot_tp_richcompare typeobject.c:1473
    #6 0x10c0edc46 in PyObject_RichCompare object.c:671
    #7 0x10c0ee101 in PyObject_RichCompareBool object.c:741
    #8 0x1100dc593 in _asyncio_Future_remove_done_callback _asynciomodule.c:531
    #9 0x10c0403e3 in _PyMethodDef_RawFastCallKeywords call.c:630
    #10 0x10c04f659 in _PyMethodDescr_FastCallKeywords descrobject.c:287
    #11 0x10c27f77f in call_function ceval.c:4854
    #12 0x10c26b0e8 in _PyEval_EvalFrameDefault ceval.c:3336
    #13 0x10c281b6b in _PyEval_EvalCodeWithName ceval.c:678
    #14 0x10c264603 in PyEval_EvalCode ceval.c:4221
    #15 0x10c3175f7 in PyRun_FileExFlags pythonrun.c:982
    #16 0x10c315cc0 in PyRun_SimpleFileExFlags pythonrun.c:398
    #17 0x10c3622b5 in Py_Main main.c:341
    #18 0x10bfe6a60 in main python.c:102
    #19 0x7fff96740234 in start (libdyld.dylib+0x5234)

    In order to fix this, I recommend just banning returning an error when the user attempts to append to the done callbacks while removing one. See the attached patch (may need fixing for style/consistency with error messages).

    @NedWilliamson NedWilliamson mannequin added 3.7 (EOL) end of life topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 2, 2017
    @1st1
    Copy link
    Member

    1st1 commented Jul 5, 2017

    New changeset 833a3b0 by Yury Selivanov in branch 'master':
    bpo-30828: Fix out of bounds write in `asyncio.CFuture.remove_done_callback() (bpo-2569)
    833a3b0

    @1st1
    Copy link
    Member

    1st1 commented Jul 5, 2017

    New changeset aaa4f99 by Yury Selivanov in branch '3.6':
    [3.6] bpo-30828: Fix out of bounds write in `asyncio.CFuture.remove_done_callback() (GH-2569) (bpo-2590)
    aaa4f99

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @1st1 @serhiy-storchaka