assertion failures in ctypes

[orenmn]

BPO	28129
Nosy	@vsajip, @vstinner, @orenmn
PRs	bpo-28129: fix ctypes crashes #386 bpo-28129: Add NULL checks #403 [3.6] bpo-28129: fix ctypes crashes #3799 [2.7] bpo-28129: fix ctypes crashes #3800
Files	testBugsOrPatches.py: an ugly script that verifies the bugs or the patches issue28129_ver1.diff: proposed patches diff file - ver1 CPythonTestOutput.txt: test output of CPython without my patches (tested on my PC) patchedCPythonTestOutput_ver1.txt: test output of CPython with my patches (tested on my PC) - ver1

^{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-28.14:33:09.216>
created_at = <Date 2016-09-13.15:14:14.619>
labels = ['ctypes', '3.7', 'type-crash']
title = 'assertion failures in ctypes'
updated_at = <Date 2017-09-28.14:33:09.215>
user = 'https://github.com/orenmn'

bugs.python.org fields:

activity = <Date 2017-09-28.14:33:09.215>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-28.14:33:09.216>
closer = 'vstinner'
components = ['ctypes']
creation = <Date 2016-09-13.15:14:14.619>
creator = 'Oren Milman'
dependencies = []
files = ['44636', '44637', '44638', '44639']
hgrepos = []
issue_num = 28129
keywords = ['patch']
message_count = 8.0
messages = ['276288', '288778', '290342', '303235', '303239', '303243', '303244', '303245']
nosy_count = 3.0
nosy_names = ['vinay.sajip', 'vstinner', 'Oren Milman']
pr_nums = ['386', '403', '3799', '3800']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue28129'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

[orenmn]

------------ current state ------------
In Modules\_ctypes\_ctypes.c, there are six functions with assertions that might fail:
1. CDataType_from_buffer
2. CDataType_from_buffer_copy
3. PyCPointerType_set_type
4. PyCPointerType_from_param
5. PyCSimpleType_from_param
6. _validate_paramflags
The following is true for each of these functions:
- It assumes its first argument is a subclass (or an instance of a subclass) of some abstract ctype, which means it (the first argument) has a storage dict.
- Thus, it asserts its first argument has a storage dict.
- However, its first argument might be some abstract ctype (and not a subclass (or an instance of a subclass) of that abstract ctype), in which case the assertion fails.

In Modules\ctypes\cfield.c, there are two functions with assertions that might fail:
1. PyCField_set
2. PyCField_get
These functions are the C implementations of the __set_ and __get__ functions (respectively) of the CFeild type. Each of them asserts its instance argument is a CDataObject, which might not be true.

------------ proposed changes ------------
Replace each of these assertions with an if statement that raises an exception in case of an invalid argument.

------------ diff ------------
The proposed patches diff file is attached.

------------ tests ------------
I wrote an ugly script to verify the assertion failures on CPython without my patches, and to test the patches on CPython with my patches. The script is attached, but it would probably fail on a non-Windows machine.

I built the patched CPython for x86, and played with it a little. Everything seemed to work as usual.

In addition, I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with and without the patches, and got quite the same output.
The outputs of both runs are attached.

[orenmn]

The fix for issue bpo-25659 already replaced the assertions in
CDataType_from_buffer and CDataType_from_buffer_copy with if statements (my
bad for missing that issue when I opened this one).
In addition, that fix added some tests, so I also added some, and created a
pull request.

I run the test module again, and on my Windows 10, the same tests failed with
and without my patches. However, on my Ubuntu 16.04 VM, none of the tests
failed.

[vstinner]

New changeset 1bea762 by Victor Stinner (orenmn) in branch 'master':
bpo-28129: fix ctypes crashes (#386)
1bea762

[orenmn]

Shouldn't we close this issue?

[vstinner]

Shouldn't we close this issue?

Oh, I forgot this issue.

Python 2.7 and 3.6 are also impacted and still accept bug fixes. I proposed backports.

[vstinner]

New changeset 8b83687 by Victor Stinner in branch '2.7':
bpo-28129: fix ctypes crashes (#386) (bpo-3800)
8b83687

[vstinner]

New changeset 7d6ddb9 by Victor Stinner in branch '3.6':
bpo-28129: fix ctypes crashes (#386) (bpo-3799)
7d6ddb9

[vstinner]

Ok, now the issue can be closed :-) I backported Oren Milman's fix to Python 2.7 and 3.6 as well.

Oren Milman: thank you very much for your fix!