From ba7d7365215d791025d1efd25393c91404f2cfc8 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Tue, 29 Aug 2017 11:58:27 +0300 Subject: [PATCH] bpo-31243: Fixed PyArg_ParseTuple failure checks. (#3171) --- Lib/test/test_io.py | 20 ++++++++ .../2017-08-24-13-34-49.bpo-31243.dRJzqR.rst | 2 + Modules/_io/textio.c | 18 ++++++-- Modules/_testcapimodule.c | 46 ++++++++++++------- 4 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index ba95c1442fa..48270c88be6 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -3253,6 +3253,26 @@ class TextIOWrapperTest(unittest.TestCase): t = _make_illegal_wrapper() self.assertRaises(TypeError, t.read) + # Issue 31243: calling read() while the return value of decoder's + # getstate() is invalid should neither crash the interpreter nor + # raise a SystemError. + def _make_very_illegal_wrapper(getstate_ret_val): + class BadDecoder: + def getstate(self): + return getstate_ret_val + def _get_bad_decoder(dummy): + return BadDecoder() + quopri = codecs.lookup("quopri") + with support.swap_attr(quopri, 'incrementaldecoder', + _get_bad_decoder): + return _make_illegal_wrapper() + t = _make_very_illegal_wrapper(42) + self.assertRaises(TypeError, t.read, 42) + t = _make_very_illegal_wrapper(()) + self.assertRaises(TypeError, t.read, 42) + t = _make_very_illegal_wrapper((1, 2)) + self.assertRaises(TypeError, t.read, 42) + def _check_create_at_shutdown(self, **kwargs): # Issue #20037: creating a TextIOWrapper at shutdown # shouldn't crash the interpreter. diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst new file mode 100644 index 00000000000..166458f2b78 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst @@ -0,0 +1,2 @@ +Fix a crash in some methods of `io.TextIOWrapper`, when the decoder's state +is invalid. Patch by Oren Milman. diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 402f743c61f..9b7334d3b54 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -1513,15 +1513,23 @@ textiowrapper_read_chunk(textio *self, Py_ssize_t size_hint) /* Given this, we know there was a valid snapshot point * len(dec_buffer) bytes ago with decoder state (b'', dec_flags). */ - if (PyArg_ParseTuple(state, "OO", &dec_buffer, &dec_flags) < 0) { + if (!PyTuple_Check(state)) { + PyErr_SetString(PyExc_TypeError, + "illegal decoder state"); + Py_DECREF(state); + return -1; + } + if (!PyArg_ParseTuple(state, + "OO;illegal decoder state", &dec_buffer, &dec_flags)) + { Py_DECREF(state); return -1; } if (!PyBytes_Check(dec_buffer)) { PyErr_Format(PyExc_TypeError, - "decoder getstate() should have returned a bytes " - "object, not '%.200s'", + "illegal decoder state: the first item should be a " + "bytes object, not '%.200s'", Py_TYPE(dec_buffer)->tp_name); Py_DECREF(state); return -1; @@ -2408,8 +2416,8 @@ _io_TextIOWrapper_tell_impl(textio *self) } \ if (!PyBytes_Check(dec_buffer)) { \ PyErr_Format(PyExc_TypeError, \ - "decoder getstate() should have returned a bytes " \ - "object, not '%.200s'", \ + "illegal decoder state: the first item should be a " \ + "bytes object, not '%.200s'", \ Py_TYPE(dec_buffer)->tp_name); \ Py_DECREF(_state); \ goto fail; \ diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 95c10184159..2952317e76d 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -864,8 +864,9 @@ test_L_code(PyObject *self) PyTuple_SET_ITEM(tuple, 0, num); value = -1; - if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0) + if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) { return NULL; + } if (value != 42) return raiseTestError("test_L_code", "L code returned wrong value for long 42"); @@ -878,8 +879,9 @@ test_L_code(PyObject *self) PyTuple_SET_ITEM(tuple, 0, num); value = -1; - if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0) + if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) { return NULL; + } if (value != 42) return raiseTestError("test_L_code", "L code returned wrong value for int 42"); @@ -1195,8 +1197,9 @@ test_k_code(PyObject *self) PyTuple_SET_ITEM(tuple, 0, num); value = 0; - if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0) + if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) { return NULL; + } if (value != ULONG_MAX) return raiseTestError("test_k_code", "k code returned wrong value for long 0xFFF...FFF"); @@ -1215,8 +1218,9 @@ test_k_code(PyObject *self) PyTuple_SET_ITEM(tuple, 0, num); value = 0; - if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0) + if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) { return NULL; + } if (value != (unsigned long)-0x42) return raiseTestError("test_k_code", "k code returned wrong value for long -0xFFF..000042"); @@ -1549,11 +1553,13 @@ test_s_code(PyObject *self) /* These two blocks used to raise a TypeError: * "argument must be string without null bytes, not str" */ - if (PyArg_ParseTuple(tuple, "s:test_s_code1", &value) < 0) - return NULL; + if (!PyArg_ParseTuple(tuple, "s:test_s_code1", &value)) { + return NULL; + } - if (PyArg_ParseTuple(tuple, "z:test_s_code2", &value) < 0) - return NULL; + if (!PyArg_ParseTuple(tuple, "z:test_s_code2", &value)) { + return NULL; + } Py_DECREF(tuple); Py_RETURN_NONE; @@ -1655,14 +1661,16 @@ test_u_code(PyObject *self) PyTuple_SET_ITEM(tuple, 0, obj); value = 0; - if (PyArg_ParseTuple(tuple, "u:test_u_code", &value) < 0) + if (!PyArg_ParseTuple(tuple, "u:test_u_code", &value)) { return NULL; + } if (value != PyUnicode_AS_UNICODE(obj)) return raiseTestError("test_u_code", "u code returned wrong value for u'test'"); value = 0; - if (PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len) < 0) + if (!PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len)) { return NULL; + } if (value != PyUnicode_AS_UNICODE(obj) || len != PyUnicode_GET_SIZE(obj)) return raiseTestError("test_u_code", @@ -1694,8 +1702,9 @@ test_Z_code(PyObject *self) value2 = PyUnicode_AS_UNICODE(obj); /* Test Z for both values */ - if (PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2) < 0) + if (!PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2)) { return NULL; + } if (value1 != PyUnicode_AS_UNICODE(obj)) return raiseTestError("test_Z_code", "Z code returned wrong value for 'test'"); @@ -1709,9 +1718,11 @@ test_Z_code(PyObject *self) len2 = -1; /* Test Z# for both values */ - if (PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1, - &value2, &len2) < 0) + if (!PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1, + &value2, &len2)) + { return NULL; + } if (value1 != PyUnicode_AS_UNICODE(obj) || len1 != PyUnicode_GET_SIZE(obj)) return raiseTestError("test_Z_code", @@ -2033,8 +2044,9 @@ test_empty_argparse(PyObject *self) tuple = PyTuple_New(0); if (!tuple) return NULL; - if ((result = PyArg_ParseTuple(tuple, "|:test_empty_argparse")) < 0) + if (!(result = PyArg_ParseTuple(tuple, "|:test_empty_argparse"))) { goto done; + } dict = PyDict_New(); if (!dict) goto done; @@ -2042,8 +2054,9 @@ test_empty_argparse(PyObject *self) done: Py_DECREF(tuple); Py_XDECREF(dict); - if (result < 0) + if (!result) { return NULL; + } else { Py_RETURN_NONE; } @@ -3698,8 +3711,9 @@ test_raise_signal(PyObject* self, PyObject *args) { int signum, err; - if (PyArg_ParseTuple(args, "i:raise_signal", &signum) < 0) + if (!PyArg_ParseTuple(args, "i:raise_signal", &signum)) { return NULL; + } err = raise(signum); if (err)