This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Out of bounds write in _asyncio_Future_remove_done_callback
Type: crash Stage: resolved
Components: asyncio Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ned Williamson, yselivanov
Priority: normal Keywords: patch

Created on 2017-07-02 12:24 by Ned Williamson, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix.patch Ned Williamson, 2017-07-02 12:24
Pull Requests
URL Status Linked Edit
PR 2569 merged yselivanov, 2017-07-04 17:13
PR 2590 merged yselivanov, 2017-07-05 17:34
Messages (3)
msg297513 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-07-02 12:24
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).
msg297774 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-07-05 17:32
New changeset 833a3b0d3707200daeaccdd218e8f18a190284aa by Yury Selivanov in branch 'master':
bpo-30828: Fix out of bounds write in `asyncio.CFuture.remove_done_callback() (#2569)
https://github.com/python/cpython/commit/833a3b0d3707200daeaccdd218e8f18a190284aa
msg297775 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-07-05 18:03
New changeset aaa4f991518611d101fba1ef3ecb18d7b385ad5b by Yury Selivanov in branch '3.6':
[3.6] bpo-30828: Fix out of bounds write in `asyncio.CFuture.remove_done_callback() (GH-2569) (#2590)
https://github.com/python/cpython/commit/aaa4f991518611d101fba1ef3ecb18d7b385ad5b
History
Date User Action Args
2022-04-11 14:58:48adminsetgithub: 75011
2017-09-03 05:59:17serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: resolved
2017-07-05 18:03:12yselivanovsetmessages: + msg297775
2017-07-05 17:34:04yselivanovsetpull_requests: + pull_request2660
2017-07-05 17:32:05yselivanovsetmessages: + msg297774
2017-07-04 17:13:35yselivanovsetpull_requests: + pull_request2640
2017-07-02 12:24:37Ned Williamsoncreate