Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Do not crash on recursive .dtype attribute lookup. #13982

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 47 additions & 20 deletions numpy/core/src/multiarray/descriptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,31 +77,51 @@ _arraydescr_from_ctypes_type(PyTypeObject *type)
* and it can be converted to a dtype object.
*
* Returns a new reference to a dtype object, or NULL
* if this is not possible. When it returns NULL, it does
* not set a Python exception.
* if this is not possible.
* When the return value is true, the dtype attribute should have been used
* and parsed. Currently the only failure mode for a 1 return is a
* RecursionError and the descriptor is set to NULL.
* When the return value is false, no error will be set.
*/
NPY_NO_EXPORT PyArray_Descr *
_arraydescr_from_dtype_attr(PyObject *obj)
int
_arraydescr_from_dtype_attr(PyObject *obj, PyArray_Descr **newdescr)
{
PyObject *dtypedescr;
PyArray_Descr *newdescr = NULL;
int ret;

/* For arbitrary objects that have a "dtype" attribute */
dtypedescr = PyObject_GetAttrString(obj, "dtype");
PyErr_Clear();
if (dtypedescr == NULL) {
return NULL;
/*
* This can be reached due to recursion limit being hit while fetching
* the attribute (tested for py3.7). This removes the custom message.
*/
goto fail;
}

ret = PyArray_DescrConverter(dtypedescr, &newdescr);
if (Py_EnterRecursiveCall(
" while trying to convert the given data type from its "
"`.dtype` attribute.") != 0) {
return 1;
}

ret = PyArray_DescrConverter(dtypedescr, newdescr);

Py_DECREF(dtypedescr);
Py_LeaveRecursiveCall();
if (ret != NPY_SUCCEED) {
PyErr_Clear();
return NULL;
goto fail;
}

return newdescr;
return 1;

fail:
/* Ignore all but recursion errors, to give ctypes a full try. */
if (!PyErr_ExceptionMatches(PyExc_RecursionError)) {
PyErr_Clear();
return 0;
}
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

Compilation is failing on Python2.7 since PyExc_RecursionError does not exist. Py_EnterRecursiveCall raises PyExc_RuntimeError instead

}

/*
Expand Down Expand Up @@ -1412,11 +1432,16 @@ PyArray_DescrConverter(PyObject *obj, PyArray_Descr **at)
check_num = NPY_VOID;
}
else {
*at = _arraydescr_from_dtype_attr(obj);
if (*at) {
if (_arraydescr_from_dtype_attr(obj, at)) {
/*
* Using dtype attribute, *at may be NULL if a
* RecursionError occurred.
*/
if (*at == NULL) {
goto error;
}
return NPY_SUCCEED;
}

/*
* Note: this comes after _arraydescr_from_dtype_attr because the ctypes
* type might override the dtype if numpy does not otherwise
Expand Down Expand Up @@ -1595,14 +1620,16 @@ PyArray_DescrConverter(PyObject *obj, PyArray_Descr **at)
goto fail;
}
else {
*at = _arraydescr_from_dtype_attr(obj);
if (*at) {
if (_arraydescr_from_dtype_attr(obj, at)) {
/*
* Using dtype attribute, *at may be NULL if a
* RecursionError occurred.
*/
if (*at == NULL) {
goto error;
}
return NPY_SUCCEED;
}
if (PyErr_Occurred()) {
return NPY_FAIL;
}

/*
* Note: this comes after _arraydescr_from_dtype_attr because the ctypes
* type might override the dtype if numpy does not otherwise
Expand Down
4 changes: 2 additions & 2 deletions numpy/core/src/multiarray/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ NPY_NO_EXPORT PyObject *arraydescr_protocol_descr_get(PyArray_Descr *self);
NPY_NO_EXPORT PyObject *
array_set_typeDict(PyObject *NPY_UNUSED(ignored), PyObject *args);

NPY_NO_EXPORT PyArray_Descr *
_arraydescr_from_dtype_attr(PyObject *obj);
int
_arraydescr_from_dtype_attr(PyObject *obj, PyArray_Descr **newdescr);


NPY_NO_EXPORT int
Expand Down
14 changes: 10 additions & 4 deletions numpy/core/src/multiarray/scalarapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,18 @@ PyArray_DescrFromTypeObject(PyObject *type)
/* Do special thing for VOID sub-types */
if (PyType_IsSubtype((PyTypeObject *)type, &PyVoidArrType_Type)) {
new = PyArray_DescrNewFromType(NPY_VOID);
conv = _arraydescr_from_dtype_attr(type);
if (conv) {
if (new == NULL) {
return NULL;
}
if (_arraydescr_from_dtype_attr(type, &conv)) {
if (conv == NULL) {
Py_DECREF(new);
return NULL;
}
new->fields = conv->fields;
Py_INCREF(new->fields);
Py_XINCREF(new->fields);
new->names = conv->names;
Py_INCREF(new->names);
Py_XINCREF(new->names);
new->elsize = conv->elsize;
new->subarray = conv->subarray;
conv->subarray = NULL;
Expand Down
44 changes: 44 additions & 0 deletions numpy/core/tests/test_dtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,50 @@ def test_invalid_dtype_string():
assert_raises(TypeError, np.dtype, u'Fl\xfcgel')


class TestFromDTypeAttribute(object):
def test_simple(self):
class dt:
dtype = "f8"

assert np.dtype(dt) == np.float64
assert np.dtype(dt()) == np.float64

def test_recursion(self):
class dt:
pass

dt.dtype = dt
with pytest.raises(RecursionError):
np.dtype(dt)

dt_instance = dt()
dt_instance.dtype = dt
with pytest.raises(RecursionError):
np.dtype(dt_instance)

def test_void_subtype(self):
class dt(np.void):
# This code path is fully untested before, so it is unclear
# what this should be useful for. Note that if np.void is used
# numpy will think we are deallocating a base type [1.17, 2019-02].
dtype = np.dtype("f,f")
pass

np.dtype(dt)
np.dtype(dt(1))

def test_void_subtype_recursion(self):
class dt(np.void):
pass

dt.dtype = dt

with pytest.raises(RecursionError):
np.dtype(dt)

with pytest.raises(RecursionError):
np.dtype(dt(1))

class TestFromCTypes(object):

@staticmethod
Expand Down