Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Lib/test/test_ctypes/test_prototypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,32 @@ def test_paramflags(self):
self.assertEqual(func(None), None)
self.assertEqual(func(input=None), None)

def test_invalid_paramflags(self):
proto = CFUNCTYPE(c_int, c_char_p)
with self.assertRaises(ValueError):
func = proto(("myprintf", testdll), ((1, "fmt"), (1, "arg1")))

def test_invalid_setattr_argtypes(self):
proto = CFUNCTYPE(c_int, c_char_p)
func = proto(("myprintf", testdll), ((1, "fmt"),))

with self.assertRaisesRegex(TypeError, "_argtypes_ must be a sequence of types"):
func.argtypes = 123
self.assertEqual(func.argtypes, (c_char_p,))

with self.assertRaisesRegex(ValueError, "paramflags must have the same length as argtypes"):
func.argtypes = (c_char_p, c_int)
self.assertEqual(func.argtypes, (c_char_p,))

def test_paramflags_outarg(self):
proto = CFUNCTYPE(c_int, c_char_p, c_int)
with self.assertRaisesRegex(TypeError, "must be a pointer type"):
func = proto(("myprintf", testdll), ((1, "fmt"), (2, "out")))

proto = CFUNCTYPE(c_int, c_char_p, c_void_p)
func = proto(("myprintf", testdll), ((1, "fmt"), (2, "out")))
with self.assertRaisesRegex(TypeError, "must be a pointer type"):
func.argtypes = (c_char_p, c_int)

def test_int_pointer_arg(self):
func = testdll._testfunc_p_p
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix segmentation faults in the :mod:`ctypes` module due to invalid :attr:`~ctypes._CFuncPtr.argtypes`. Patch by Dung Nguyen.
33 changes: 22 additions & 11 deletions Modules/_ctypes/_ctypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3647,6 +3647,9 @@ atomic_xgetref(PyObject *obj, PyObject **field)
#endif
}

static int
_validate_paramflags(ctypes_state *st, PyTypeObject *type, PyObject *paramflags, PyObject *argtypes);



/*[clinic input]
Expand Down Expand Up @@ -3760,16 +3763,22 @@ static int
_ctypes_CFuncPtr_argtypes_set_impl(PyCFuncPtrObject *self, PyObject *value)
/*[clinic end generated code: output=596a36e2ae89d7d1 input=c4627573e980aa8b]*/
{
PyObject *converters;

if (value == NULL || value == Py_None) {
atomic_xsetref(&self->argtypes, NULL);
atomic_xsetref(&self->converters, NULL);
} else {
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
converters = converters_from_argtypes(st, value);
PyTypeObject *type = Py_TYPE(self);
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));

PyObject *converters = converters_from_argtypes(st, value);
if (!converters)
return -1;

/* Verify paramflags again due to constraints with argtypes */
if (!_validate_paramflags(st, type, self->paramflags, value)) {
Py_DECREF(converters);
return -1;
}
atomic_xsetref(&self->converters, converters);
Py_INCREF(value);
atomic_xsetref(&self->argtypes, value);
Expand Down Expand Up @@ -3899,10 +3908,9 @@ _check_outarg_type(ctypes_state *st, PyObject *arg, Py_ssize_t index)

/* Returns 1 on success, 0 on error */
static int
_validate_paramflags(ctypes_state *st, PyTypeObject *type, PyObject *paramflags)
_validate_paramflags(ctypes_state *st, PyTypeObject *type, PyObject *paramflags, PyObject *argtypes)
{
Py_ssize_t i, len;
PyObject *argtypes;

StgInfo *info;
if (PyStgInfo_FromType(st, (PyObject *)type, &info) < 0) {
Expand All @@ -3913,10 +3921,13 @@ _validate_paramflags(ctypes_state *st, PyTypeObject *type, PyObject *paramflags)
"abstract class");
return 0;
}
argtypes = info->argtypes;
if (argtypes == NULL || argtypes == Py_None) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to move the Py_None check to _ctypes_CFuncPtr_argtypes_set_impl? It would be nice to have this closer to the “user”, so the internals only need to consider NULL as special.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the Py_None check since it's already handled by _ctypes_CFuncPtr_argtypes_set_impl. The argtypes parameter will either be NULL or a non-None value.

argtypes = info->argtypes;
}

if (paramflags == NULL || info->argtypes == NULL)
if (paramflags == NULL || argtypes == NULL) {
return 1;
}

if (!PyTuple_Check(paramflags)) {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -3925,7 +3936,7 @@ _validate_paramflags(ctypes_state *st, PyTypeObject *type, PyObject *paramflags)
}

len = PyTuple_GET_SIZE(paramflags);
if (len != PyTuple_GET_SIZE(info->argtypes)) {
if (len != PyTuple_GET_SIZE(argtypes)) {
PyErr_SetString(PyExc_ValueError,
"paramflags must have the same length as argtypes");
return 0;
Expand Down Expand Up @@ -4101,7 +4112,7 @@ PyCFuncPtr_FromDll(PyTypeObject *type, PyObject *args, PyObject *kwds)
#endif
#undef USE_DLERROR
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
if (!_validate_paramflags(st, type, paramflags)) {
if (!_validate_paramflags(st, type, paramflags, NULL)) {
Py_DECREF(ftuple);
return NULL;
}
Expand Down Expand Up @@ -4145,7 +4156,7 @@ PyCFuncPtr_FromVtblIndex(PyTypeObject *type, PyObject *args, PyObject *kwds)
paramflags = NULL;

ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
if (!_validate_paramflags(st, type, paramflags)) {
if (!_validate_paramflags(st, type, paramflags, NULL)) {
return NULL;
}
self = (PyCFuncPtrObject *)generic_pycdata_new(st, type, args, kwds);
Expand Down
Loading