Out of bounds write in _asyncio_Future_remove_done_callback

[NedWilliamson]

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]

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).

[1st1]

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]

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