diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 6e87370c2065ba..f1c7a3dc8f15ee 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3482,6 +3482,79 @@ def __init__(self): pass self.assertRaises(pickle.PicklingError, BadPickler().dump, 0) self.assertRaises(pickle.UnpicklingError, BadUnpickler().load) + def test_unpickler_bad_file(self): + # bpo-38384: Crash in _pickle if the read attribute raises an error. + def raises_oserror(self, *args, **kwargs): + raise OSError + @property + def bad_property(self): + 1/0 + + # File without read + class F: + readline = raises_oserror + self.assertRaises((AttributeError, TypeError), self.Unpickler, F()) + + # File without readline + class F: + read = raises_oserror + self.assertRaises((AttributeError, TypeError), self.Unpickler, F()) + + # File with bad read + class F: + read = bad_property + readline = raises_oserror + self.assertRaises(ZeroDivisionError, self.Unpickler, F()) + + # File with bad readline + class F: + readline = bad_property + read = raises_oserror + self.assertRaises(ZeroDivisionError, self.Unpickler, F()) + + # File with bad read and without readline + class F: + read = bad_property + self.assertRaises(ZeroDivisionError, self.Unpickler, F()) + + # File with bad readline and without read + class F: + readline = bad_property + self.assertRaises(ZeroDivisionError, self.Unpickler, F()) + + # File with bad peek + class F: + peek = bad_property + read = raises_oserror + readline = raises_oserror + try: + self.Unpickler(F()) + except ZeroDivisionError: + pass + + # File with bad readinto + class F: + readinto = bad_property + read = raises_oserror + readline = raises_oserror + try: + self.Unpickler(F()) + except ZeroDivisionError: + pass + + def test_pickler_bad_file(self): + # File without write + class F: + pass + self.assertRaises(TypeError, self.Pickler, F()) + + # File with bad write + class F: + @property + def write(self): + 1/0 + self.assertRaises(ZeroDivisionError, self.Pickler, F()) + def check_dumps_loads_oob_buffers(self, dumps, loads): # No need to do the full gamut of tests here, just enough to # check that dumps() and loads() redirect their arguments diff --git a/Misc/NEWS.d/next/Tests/2023-04-03-06-16-37.gh-issue-82565.6SpJ0f.rst b/Misc/NEWS.d/next/Tests/2023-04-03-06-16-37.gh-issue-82565.6SpJ0f.rst new file mode 100644 index 00000000000000..bfb76c2dee7c0e --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2023-04-03-06-16-37.gh-issue-82565.6SpJ0f.rst @@ -0,0 +1,2 @@ +Fix a crash in the C implementation of :mod:`pickle` if an error occurred +while looking up the ``read`` and ``readline`` attribute of the file. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index a26732af8ba2a1..f8639ef8031346 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -1632,6 +1632,11 @@ _Unpickler_New(void) static int _Unpickler_SetInputStream(UnpicklerObject *self, PyObject *file) { + _Py_IDENTIFIER(peek); + _Py_IDENTIFIER(read); + _Py_IDENTIFIER(readinto); + _Py_IDENTIFIER(readline); + /* Optional file methods */ if (_PyObject_LookupAttr(file, &_Py_ID(peek), &self->peek) < 0) { return -1; @@ -1639,9 +1644,9 @@ _Unpickler_SetInputStream(UnpicklerObject *self, PyObject *file) if (_PyObject_LookupAttr(file, &_Py_ID(readinto), &self->readinto) < 0) { return -1; } - (void)_PyObject_LookupAttr(file, &_Py_ID(read), &self->read); - (void)_PyObject_LookupAttr(file, &_Py_ID(readline), &self->readline); - if (!self->readline || !self->read) { + if (_PyObject_LookupAttrId(file, &PyId_read, &self->read) <= 0 || + _PyObject_LookupAttrId(file, &PyId_readline, &self->readline) <= 0) + { if (!PyErr_Occurred()) { PyErr_SetString(PyExc_TypeError, "file must have 'read' and 'readline' attributes");