From 7d6ddb96b34b94c1cbdf95baa94492c48426404e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 28 Sep 2017 07:32:11 -0700 Subject: [PATCH] bpo-28129: fix ctypes crashes (#386) (#3799) * init commit, with initial tests for from_param and fields __set__ and __get__, and some additions to from_buffer and from_buffer_copy * added the rest of tests and patches. probably only a first draft. * removed trailing spaces * replace ctype with ctypes in error messages * change back from ctypes instance to ctype instance (cherry picked from commit 1bea762d9ec823544c530d567330a47f64d93d4f) --- Lib/ctypes/test/test_frombuffer.py | 9 ++++++++ Lib/ctypes/test/test_funcptr.py | 5 +++++ Lib/ctypes/test/test_parameters.py | 10 +++++++++ Lib/ctypes/test/test_pointers.py | 5 +++++ Lib/ctypes/test/test_struct_fields.py | 24 +++++++++++++++++++++ Modules/_ctypes/_ctypes.c | 31 +++++++++++++++++++++------ Modules/_ctypes/cfield.c | 12 +++++++++-- 7 files changed, 87 insertions(+), 9 deletions(-) diff --git a/Lib/ctypes/test/test_frombuffer.py b/Lib/ctypes/test/test_frombuffer.py index 7ab38f1b03..55c244356b 100644 --- a/Lib/ctypes/test/test_frombuffer.py +++ b/Lib/ctypes/test/test_frombuffer.py @@ -121,12 +121,21 @@ class Test(unittest.TestCase): (c_int * 1).from_buffer_copy(a, 16 * sizeof(c_int)) def test_abstract(self): + from ctypes import _Pointer, _SimpleCData, _CFuncPtr + self.assertRaises(TypeError, Array.from_buffer, bytearray(10)) self.assertRaises(TypeError, Structure.from_buffer, bytearray(10)) self.assertRaises(TypeError, Union.from_buffer, bytearray(10)) + self.assertRaises(TypeError, _CFuncPtr.from_buffer, bytearray(10)) + self.assertRaises(TypeError, _Pointer.from_buffer, bytearray(10)) + self.assertRaises(TypeError, _SimpleCData.from_buffer, bytearray(10)) + self.assertRaises(TypeError, Array.from_buffer_copy, b"123") self.assertRaises(TypeError, Structure.from_buffer_copy, b"123") self.assertRaises(TypeError, Union.from_buffer_copy, b"123") + self.assertRaises(TypeError, _CFuncPtr.from_buffer_copy, b"123") + self.assertRaises(TypeError, _Pointer.from_buffer_copy, b"123") + self.assertRaises(TypeError, _SimpleCData.from_buffer_copy, b"123") if __name__ == '__main__': unittest.main() diff --git a/Lib/ctypes/test/test_funcptr.py b/Lib/ctypes/test/test_funcptr.py index f34734b164..e0b9b54e97 100644 --- a/Lib/ctypes/test/test_funcptr.py +++ b/Lib/ctypes/test/test_funcptr.py @@ -123,5 +123,10 @@ class CFuncPtrTestCase(unittest.TestCase): self.assertEqual(strtok(None, b"\n"), b"c") self.assertEqual(strtok(None, b"\n"), None) + def test_abstract(self): + from ctypes import _CFuncPtr + + self.assertRaises(TypeError, _CFuncPtr, 13, "name", 42, "iid") + if __name__ == '__main__': unittest.main() diff --git a/Lib/ctypes/test/test_parameters.py b/Lib/ctypes/test/test_parameters.py index a61f1c7fa7..e4c25fd880 100644 --- a/Lib/ctypes/test/test_parameters.py +++ b/Lib/ctypes/test/test_parameters.py @@ -170,6 +170,16 @@ class SimpleTypesTestCase(unittest.TestCase): # ArgumentError: argument 1: ValueError: 99 self.assertRaises(ArgumentError, func, 99) + def test_abstract(self): + from ctypes import (Array, Structure, Union, _Pointer, + _SimpleCData, _CFuncPtr) + + self.assertRaises(TypeError, Array.from_param, 42) + self.assertRaises(TypeError, Structure.from_param, 42) + self.assertRaises(TypeError, Union.from_param, 42) + self.assertRaises(TypeError, _CFuncPtr.from_param, 42) + self.assertRaises(TypeError, _Pointer.from_param, 42) + self.assertRaises(TypeError, _SimpleCData.from_param, 42) @test.support.cpython_only def test_issue31311(self): diff --git a/Lib/ctypes/test/test_pointers.py b/Lib/ctypes/test/test_pointers.py index 751f85fd11..e97515879f 100644 --- a/Lib/ctypes/test/test_pointers.py +++ b/Lib/ctypes/test/test_pointers.py @@ -213,6 +213,11 @@ class PointersTestCase(unittest.TestCase): from ctypes import _pointer_type_cache del _pointer_type_cache[id(P)] + def test_abstract(self): + from ctypes import _Pointer + + self.assertRaises(TypeError, _Pointer.set_type, 42) + if __name__ == '__main__': unittest.main() diff --git a/Lib/ctypes/test/test_struct_fields.py b/Lib/ctypes/test/test_struct_fields.py index 22eb3b0cd7..8045cc8267 100644 --- a/Lib/ctypes/test/test_struct_fields.py +++ b/Lib/ctypes/test/test_struct_fields.py @@ -46,5 +46,29 @@ class StructFieldsTestCase(unittest.TestCase): Y._fields_ = [] self.assertRaises(AttributeError, setattr, X, "_fields_", []) + # __set__ and __get__ should raise a TypeError in case their self + # argument is not a ctype instance. + def test___set__(self): + class MyCStruct(Structure): + _fields_ = (("field", c_int),) + self.assertRaises(TypeError, + MyCStruct.field.__set__, 'wrong type self', 42) + + class MyCUnion(Union): + _fields_ = (("field", c_int),) + self.assertRaises(TypeError, + MyCUnion.field.__set__, 'wrong type self', 42) + + def test___get__(self): + class MyCStruct(Structure): + _fields_ = (("field", c_int),) + self.assertRaises(TypeError, + MyCStruct.field.__get__, 'wrong type self', 42) + + class MyCUnion(Union): + _fields_ = (("field", c_int),) + self.assertRaises(TypeError, + MyCUnion.field.__get__, 'wrong type self', 42) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 27f6dd4344..258e805870 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -990,6 +990,7 @@ PyCPointerType_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (proto) { StgDictObject *itemdict = PyType_stgdict(proto); const char *current_format; + /* PyCPointerType_SetProto has verified proto has a stgdict. */ assert(itemdict); /* If itemdict->format is NULL, then this is a pointer to an incomplete type. We create a generic format string @@ -1036,7 +1037,11 @@ PyCPointerType_set_type(PyTypeObject *self, PyObject *type) StgDictObject *dict; dict = PyType_stgdict((PyObject *)self); - assert(dict); + if (!dict) { + PyErr_SetString(PyExc_TypeError, + "abstract class"); + return NULL; + } if (-1 == PyCPointerType_SetProto(dict, type)) return NULL; @@ -1062,7 +1067,11 @@ PyCPointerType_from_param(PyObject *type, PyObject *value) } typedict = PyType_stgdict(type); - assert(typedict); /* Cannot be NULL for pointer types */ + if (!typedict) { + PyErr_SetString(PyExc_TypeError, + "abstract class"); + return NULL; + } /* If we expect POINTER(), but receive a instance, accept it by calling byref(). @@ -2127,7 +2136,11 @@ PyCSimpleType_from_param(PyObject *type, PyObject *value) } dict = PyType_stgdict(type); - assert(dict); + if (!dict) { + PyErr_SetString(PyExc_TypeError, + "abstract class"); + return NULL; + } /* I think we can rely on this being a one-character string */ fmt = PyUnicode_AsUTF8(dict->proto); @@ -3231,7 +3244,11 @@ _validate_paramflags(PyTypeObject *type, PyObject *paramflags) PyObject *argtypes; dict = PyType_stgdict((PyObject *)type); - assert(dict); /* Cannot be NULL. 'type' is a PyCFuncPtr type. */ + if (!dict) { + PyErr_SetString(PyExc_TypeError, + "abstract class"); + return 0; + } argtypes = dict->argtypes; if (paramflags == NULL || dict->argtypes == NULL) @@ -4862,7 +4879,7 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) } stgdict = PyObject_stgdict((PyObject *)self); - assert(stgdict); /* Cannot be NULL fr pointer instances */ + assert(stgdict); /* Cannot be NULL for pointer instances */ proto = stgdict->proto; assert(proto); @@ -4890,7 +4907,7 @@ Pointer_get_contents(CDataObject *self, void *closure) } stgdict = PyObject_stgdict((PyObject *)self); - assert(stgdict); /* Cannot be NULL fr pointer instances */ + assert(stgdict); /* Cannot be NULL for pointer instances */ return PyCData_FromBaseObj(stgdict->proto, (PyObject *)self, 0, *(void **)self->b_ptr); @@ -4909,7 +4926,7 @@ Pointer_set_contents(CDataObject *self, PyObject *value, void *closure) return -1; } stgdict = PyObject_stgdict((PyObject *)self); - assert(stgdict); /* Cannot be NULL fr pointer instances */ + assert(stgdict); /* Cannot be NULL for pointer instances */ assert(stgdict->proto); if (!CDataObject_Check(value)) { int res = PyObject_IsInstance(value, stgdict->proto); diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 113a815db1..e39065fa93 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -205,7 +205,11 @@ PyCField_set(CFieldObject *self, PyObject *inst, PyObject *value) { CDataObject *dst; char *ptr; - assert(CDataObject_Check(inst)); + if (!CDataObject_Check(inst)) { + PyErr_SetString(PyExc_TypeError, + "not a ctype instance"); + return -1; + } dst = (CDataObject *)inst; ptr = dst->b_ptr + self->offset; if (value == NULL) { @@ -225,7 +229,11 @@ PyCField_get(CFieldObject *self, PyObject *inst, PyTypeObject *type) Py_INCREF(self); return (PyObject *)self; } - assert(CDataObject_Check(inst)); + if (!CDataObject_Check(inst)) { + PyErr_SetString(PyExc_TypeError, + "not a ctype instance"); + return NULL; + } src = (CDataObject *)inst; return PyCData_get(self->proto, self->getfunc, inst, self->index, self->size, src->b_ptr + self->offset); -- 2.40.0