From 8d6c62dd892de77295e9db7b1c56fec041726afb Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Sat, 26 Mar 2011 17:56:28 -0500 Subject: check possible recursive _as_parameter_ to prevent segfault (closes #1838) --- Lib/ctypes/test/test_as_parameter.py | 12 ++++++++++++ Lib/lib2to3/refactor.py | 2 +- Lib/lib2to3/tests/test_refactor.py | 17 +++++++++++++++++ Misc/NEWS | 3 +++ Modules/_ctypes/_ctypes.c | 11 ++++++++++- 5 files changed, 43 insertions(+), 2 deletions(-) diff --git a/Lib/ctypes/test/test_as_parameter.py b/Lib/ctypes/test/test_as_parameter.py index 835398f..475d595 100644 --- a/Lib/ctypes/test/test_as_parameter.py +++ b/Lib/ctypes/test/test_as_parameter.py @@ -187,6 +187,18 @@ class BasicWrapTestCase(unittest.TestCase): self.assertEqual((s8i.a, s8i.b, s8i.c, s8i.d, s8i.e, s8i.f, s8i.g, s8i.h), (9*2, 8*3, 7*4, 6*5, 5*6, 4*7, 3*8, 2*9)) + def test_recursive_as_param(self): + from ctypes import c_int + + class A(object): + pass + + a = A() + a._as_parameter_ = a + with self.assertRaises(RuntimeError): + c_int.from_param(a) + + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ class AsParamWrapper(object): diff --git a/Lib/lib2to3/refactor.py b/Lib/lib2to3/refactor.py index ae5e40f..ca07be6 100644 --- a/Lib/lib2to3/refactor.py +++ b/Lib/lib2to3/refactor.py @@ -500,7 +500,7 @@ class RefactoringTool(object): node = new def processed_file(self, new_text, filename, old_text=None, write=False, - encoding=None): + encoding=None, newlines=None): """ Called when a file has been refactored, and there are changes. """ diff --git a/Lib/lib2to3/tests/test_refactor.py b/Lib/lib2to3/tests/test_refactor.py index 73122d8..b6d5b57 100644 --- a/Lib/lib2to3/tests/test_refactor.py +++ b/Lib/lib2to3/tests/test_refactor.py @@ -231,6 +231,23 @@ from __future__ import print_function""" os.path.join("a_dir", "stuff.py")] check(tree, tree) + def test_preserve_file_newlines(self): + rt = self.rt(fixers=_2TO3_FIXERS) + for nl in ("\r\n", "\n"): + data = "print y%s%syes%sok%s" % ((nl,) * 4) + handle, tmp = tempfile.mkstemp() + os.close(handle) + try: + with open(tmp, "w") as fp: + fp.write(data) + rt.refactor_file(tmp) + with open(tmp, "r") as fp: + contents = fp.read() + finally: + os.unlink(tmp) + for line in contents.splitlines(True): + self.assertTrue(line.endswith(nl)) + def test_file_encoding(self): fn = os.path.join(TEST_DATA_DIR, "different_encoding.py") self.check_file_refactoring(fn) diff --git a/Misc/NEWS b/Misc/NEWS index 8e69a84..936930b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -238,6 +238,9 @@ Library Extensions ---------- +- Issue #1838: Prevent segfault in ctypes, when _as_parameter_ on a class is set + to an instance of the class. + - Issue #678250: Make mmap flush a noop on ACCESS_READ and ACCESS_COPY. Build diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 04b07cc..6a2b7ce 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -2004,10 +2004,14 @@ PyCSimpleType_from_param(PyObject *type, PyObject *value) PyCArgObject *parg; struct fielddesc *fd; PyObject *as_parameter; + int res; /* If the value is already an instance of the requested type, we can use it as is */ - if (1 == PyObject_IsInstance(value, type)) { + res = PyObject_IsInstance(value, type); + if (res == -1) + return NULL; + if (res) { Py_INCREF(value); return value; } @@ -2036,7 +2040,12 @@ PyCSimpleType_from_param(PyObject *type, PyObject *value) as_parameter = PyObject_GetAttrString(value, "_as_parameter_"); if (as_parameter) { + if (Py_EnterRecursiveCall("while processing _as_parameter_")) { + Py_DECREF(as_parameter); + return NULL; + } value = PyCSimpleType_from_param(type, as_parameter); + Py_LeaveRecursiveCall(); Py_DECREF(as_parameter); return value; } -- cgit v0.12 From 39530f8cbed8c5e08571300f7b40d268686641fc Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Sat, 26 Mar 2011 18:04:09 -0500 Subject: always check return value of PyObject_IsInstance for error --- Modules/_ctypes/_ctypes.c | 63 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 6a2b7ce..bdf7ab4 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -587,7 +587,10 @@ static PyObject * CDataType_from_param(PyObject *type, PyObject *value) { PyObject *as_parameter; - if (1 == PyObject_IsInstance(value, type)) { + int res = PyObject_IsInstance(value, type); + if (res == -1) + return NULL; + if (res) { Py_INCREF(value); return value; } @@ -600,10 +603,14 @@ CDataType_from_param(PyObject *type, PyObject *value) /* If we got a PyCArgObject, we must check if the object packed in it is an instance of the type's dict->proto */ - if(dict && ob - && PyObject_IsInstance(ob, dict->proto)) { - Py_INCREF(value); - return value; + if(dict && ob) { + res = PyObject_IsInstance(ob, dict->proto); + if (res == -1) + return NULL; + if (res) { + Py_INCREF(value); + return value; + } } ob_name = (ob) ? Py_TYPE(ob)->tp_name : "???"; PyErr_Format(PyExc_TypeError, @@ -953,8 +960,7 @@ PyCPointerType_from_param(PyObject *type, PyObject *value) Py_INCREF(value); /* _byref steals a refcount */ return _byref(value); case -1: - PyErr_Clear(); - break; + return NULL; default: break; } @@ -1445,6 +1451,7 @@ static PyObject * c_wchar_p_from_param(PyObject *type, PyObject *value) { PyObject *as_parameter; + int res; if (value == Py_None) { Py_INCREF(Py_None); return Py_None; @@ -1465,7 +1472,10 @@ c_wchar_p_from_param(PyObject *type, PyObject *value) } return (PyObject *)parg; } - if (PyObject_IsInstance(value, type)) { + res = PyObject_IsInstance(value, type); + if (res == -1) + return NULL; + if (res) { Py_INCREF(value); return value; } @@ -1506,6 +1516,7 @@ static PyObject * c_char_p_from_param(PyObject *type, PyObject *value) { PyObject *as_parameter; + int res; if (value == Py_None) { Py_INCREF(Py_None); return Py_None; @@ -1526,7 +1537,10 @@ c_char_p_from_param(PyObject *type, PyObject *value) } return (PyObject *)parg; } - if (PyObject_IsInstance(value, type)) { + res = PyObject_IsInstance(value, type); + if (res == -1) + return NULL; + if (res) { Py_INCREF(value); return value; } @@ -1568,6 +1582,7 @@ c_void_p_from_param(PyObject *type, PyObject *value) { StgDictObject *stgd; PyObject *as_parameter; + int res; /* None */ if (value == Py_None) { @@ -1645,7 +1660,10 @@ c_void_p_from_param(PyObject *type, PyObject *value) return (PyObject *)parg; } /* c_void_p instance (or subclass) */ - if (PyObject_IsInstance(value, type)) { + res = PyObject_IsInstance(value, type); + if (res == -1) + return NULL; + if (res) { /* c_void_p instances */ Py_INCREF(value); return value; @@ -2737,6 +2755,7 @@ _PyCData_set(CDataObject *dst, PyObject *type, SETFUNC setfunc, PyObject *value, Py_ssize_t size, char *ptr) { CDataObject *src; + int err; if (setfunc) return setfunc(ptr, value, size); @@ -2777,7 +2796,10 @@ _PyCData_set(CDataObject *dst, PyObject *type, SETFUNC setfunc, PyObject *value, } src = (CDataObject *)value; - if (PyObject_IsInstance(value, type)) { + err = PyObject_IsInstance(value, type); + if (err == -1) + return NULL; + if (err) { memcpy(ptr, src->b_ptr, size); @@ -4772,14 +4794,17 @@ Pointer_set_contents(CDataObject *self, PyObject *value, void *closure) stgdict = PyObject_stgdict((PyObject *)self); assert(stgdict); /* Cannot be NULL fr pointer instances */ assert(stgdict->proto); - if (!CDataObject_Check(value) - || 0 == PyObject_IsInstance(value, stgdict->proto)) { - /* XXX PyObject_IsInstance could return -1! */ - PyErr_Format(PyExc_TypeError, - "expected %s instead of %s", - ((PyTypeObject *)(stgdict->proto))->tp_name, - Py_TYPE(value)->tp_name); - return -1; + if (!CDataObject_Check(value)) { + int res = PyObject_IsInstance(value, stgdict->proto); + if (res == -1) + return -1; + if (!res) { + PyErr_Format(PyExc_TypeError, + "expected %s instead of %s", + ((PyTypeObject *)(stgdict->proto))->tp_name, + Py_TYPE(value)->tp_name); + return -1; + } } dst = (CDataObject *)value; -- cgit v0.12 From c01ffdf61e33b87c3e5e91a722e27c888ed607ad Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Sat, 26 Mar 2011 18:11:54 -0500 Subject: revert unintended changes --- Lib/lib2to3/refactor.py | 2 +- Lib/lib2to3/tests/test_refactor.py | 17 ----------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/Lib/lib2to3/refactor.py b/Lib/lib2to3/refactor.py index ca07be6..ae5e40f 100644 --- a/Lib/lib2to3/refactor.py +++ b/Lib/lib2to3/refactor.py @@ -500,7 +500,7 @@ class RefactoringTool(object): node = new def processed_file(self, new_text, filename, old_text=None, write=False, - encoding=None, newlines=None): + encoding=None): """ Called when a file has been refactored, and there are changes. """ diff --git a/Lib/lib2to3/tests/test_refactor.py b/Lib/lib2to3/tests/test_refactor.py index b6d5b57..73122d8 100644 --- a/Lib/lib2to3/tests/test_refactor.py +++ b/Lib/lib2to3/tests/test_refactor.py @@ -231,23 +231,6 @@ from __future__ import print_function""" os.path.join("a_dir", "stuff.py")] check(tree, tree) - def test_preserve_file_newlines(self): - rt = self.rt(fixers=_2TO3_FIXERS) - for nl in ("\r\n", "\n"): - data = "print y%s%syes%sok%s" % ((nl,) * 4) - handle, tmp = tempfile.mkstemp() - os.close(handle) - try: - with open(tmp, "w") as fp: - fp.write(data) - rt.refactor_file(tmp) - with open(tmp, "r") as fp: - contents = fp.read() - finally: - os.unlink(tmp) - for line in contents.splitlines(True): - self.assertTrue(line.endswith(nl)) - def test_file_encoding(self): fn = os.path.join(TEST_DATA_DIR, "different_encoding.py") self.check_file_refactoring(fn) -- cgit v0.12