bpo-28129: fix ctypes crashes (#386) (#3800)

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

diff --git a/Lib/ctypes/test/test_frombuffer.py b/Lib/ctypes/test/test_frombuffer.py
index d708ed6906fe435b7690fc53a254af8e8f513ddd..99c32e095b8d37cc8240efe0256c13047656a32e 100644 (file)
--- a/Lib/ctypes/test/test_frombuffer.py
+++ b/Lib/ctypes/test/test_frombuffer.py
@@ -78,12 +78,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 58cbb47af49085902cfc5311e3a2ad1721672aa4..575030327e0a68d82fa07cf447169696137fba16 100644 (file)
--- 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, "\n"), "c")
         self.assertEqual(strtok(None, "\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 0cac9069c6ced21726018f48fc946ba7508450e1..c6231f706b94bc8ca489ac68f3d3db80e4b4dcb9 100644 (file)
--- a/Lib/ctypes/test/test_parameters.py
+++ b/Lib/ctypes/test/test_parameters.py
@@ -175,6 +175,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 24b0546c89bf747f07f52d385038b3d9e46adcb1..4a8887c1dee60ef3bd8c010e251ed967c903cd9c 100644 (file)
--- a/Lib/ctypes/test/test_pointers.py
+++ b/Lib/ctypes/test/test_pointers.py
@@ -210,6 +210,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 22eb3b0cd7b8b9171ee20b3fc79bc59a930cfb20..8045cc82679cc1e10c2a4a0723f6da64ea9abd7d 100644 (file)
--- 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 fb7e9859db2b5b1d3548f4713124cdb968b81e19..25740ed771212c20308a183e7fdd2df9d112c6df 100644 (file)
--- a/Modules/_ctypes/_ctypes.c
+++ b/Modules/_ctypes/_ctypes.c
@@ -1036,6 +1036,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
@@ -1082,7 +1083,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;
@@ -1108,7 +1113,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(<type>), but receive a <type> instance, accept
        it by calling byref(<type>).
@@ -2226,7 +2235,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 = PyString_AsString(dict->proto);
@@ -3347,7 +3360,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)
@@ -5090,7 +5107,7 @@ Pointer_ass_item(PyObject *_self, 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);
@@ -5118,7 +5135,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);
@@ -5137,7 +5154,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 e033cd5fffe58b21c47e1f10c5e80b0bb40dbc28..6f632d0a072893ad3561c5c6fa056b738486c040 100644 (file)
--- 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);