diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index e04baefef081d4..5a7617cd9f9cbf 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -543,6 +543,22 @@ def test_readline(self): with self.open(support.TESTFN, "r") as f: self.assertRaises(TypeError, f.readline, 5.3) + def test_readline_nonsizeable(self): + # Issue #30061 + # Crash when readline() returns an object without __len__ + class R(self.IOBase): + def readline(self): + return None + self.assertRaises((TypeError, StopIteration), next, R()) + + def test_next_nonsizeable(self): + # Issue #30061 + # Crash when __next__() returns an object without __len__ + class R(self.IOBase): + def __next__(self): + return None + self.assertRaises(TypeError, R().readlines, 1) + def test_raw_bytes_io(self): f = self.BytesIO() self.write_ops(f) diff --git a/Misc/NEWS b/Misc/NEWS index 673211d72dda51..44a4b873b1454b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -32,6 +32,11 @@ Core and Builtins Library ------- +- bpo-30061: Fixed crashes in IOBase methods __next__() and readlines() when + readline() or __next__() respectively return non-sizeable object. + Fixed possible other errors caused by not checking results of PyObject_Size(), + PySequence_Size(), or PyMapping_Size(). + - bpo-30017: Allowed calling the close() method of the zip entry writer object multiple times. Writing to a closed writer now always produces a ValueError. diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index f7986d7e52f957..c8642040ae426a 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -625,7 +625,8 @@ iobase_iternext(PyObject *self) if (line == NULL) return NULL; - if (PyObject_Size(line) == 0) { + if (PyObject_Size(line) <= 0) { + /* Error or empty */ Py_DECREF(line); return NULL; } @@ -676,6 +677,7 @@ _io__IOBase_readlines_impl(PyObject *self, Py_ssize_t hint) } while (1) { + Py_ssize_t line_length; PyObject *line = PyIter_Next(it); if (line == NULL) { if (PyErr_Occurred()) { @@ -689,11 +691,14 @@ _io__IOBase_readlines_impl(PyObject *self, Py_ssize_t hint) Py_DECREF(line); goto error; } - length += PyObject_Size(line); + line_length = PyObject_Size(line); Py_DECREF(line); - - if (length > hint) + if (line_length < 0) { + goto error; + } + if (line_length > hint - length) break; + length += line_length; } Py_DECREF(it); diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 91d4f0172c3c50..248f4582c60fb4 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -722,17 +722,22 @@ getenvironment(PyObject* environment) return NULL; } - envsize = PyMapping_Length(environment); - keys = PyMapping_Keys(environment); values = PyMapping_Values(environment); if (!keys || !values) goto error; + envsize = PySequence_Fast_GET_SIZE(keys); + if (PySequence_Fast_GET_SIZE(values) != envsize) { + PyErr_SetString(PyExc_RuntimeError, + "environment changed size during iteration"); + goto error; + } + totalsize = 1; /* trailing null character */ for (i = 0; i < envsize; i++) { - PyObject* key = PyList_GET_ITEM(keys, i); - PyObject* value = PyList_GET_ITEM(values, i); + PyObject* key = PySequence_Fast_GET_ITEM(keys, i); + PyObject* value = PySequence_Fast_GET_ITEM(values, i); if (! PyUnicode_Check(key) || ! PyUnicode_Check(value)) { PyErr_SetString(PyExc_TypeError, @@ -760,8 +765,8 @@ getenvironment(PyObject* environment) end = buffer + totalsize; for (i = 0; i < envsize; i++) { - PyObject* key = PyList_GET_ITEM(keys, i); - PyObject* value = PyList_GET_ITEM(values, i); + PyObject* key = PySequence_Fast_GET_ITEM(keys, i); + PyObject* value = PySequence_Fast_GET_ITEM(values, i); if (!PyUnicode_AsUCS4(key, p, end - p, 0)) goto error; p += PyUnicode_GET_LENGTH(key); diff --git a/Modules/cjkcodecs/multibytecodec.c b/Modules/cjkcodecs/multibytecodec.c index d1da189ddd3bef..d6efc77d20c883 100644 --- a/Modules/cjkcodecs/multibytecodec.c +++ b/Modules/cjkcodecs/multibytecodec.c @@ -1670,6 +1670,9 @@ _multibytecodec_MultibyteStreamWriter_writelines(MultibyteStreamWriterObject *se if (r == -1) return NULL; } + /* PySequence_Length() can fail */ + if (PyErr_Occurred()) + return NULL; Py_RETURN_NONE; } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 2ea5e2def9acff..f4bbc8931b39fa 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -6650,7 +6650,7 @@ static PyObject * os_setgroups(PyObject *module, PyObject *groups) /*[clinic end generated code: output=3fcb32aad58c5ecd input=fa742ca3daf85a7e]*/ { - int i, len; + Py_ssize_t i, len; gid_t grouplist[MAX_GROUPS]; if (!PySequence_Check(groups)) { @@ -6658,6 +6658,9 @@ os_setgroups(PyObject *module, PyObject *groups) return NULL; } len = PySequence_Size(groups); + if (len < 0) { + return NULL; + } if (len > MAX_GROUPS) { PyErr_SetString(PyExc_ValueError, "too many groups"); return NULL; @@ -7886,9 +7889,9 @@ os_read_impl(PyObject *module, int fd, Py_ssize_t length) #if (defined(HAVE_SENDFILE) && (defined(__FreeBSD__) || defined(__DragonFly__) \ || defined(__APPLE__))) || defined(HAVE_READV) || defined(HAVE_WRITEV) static Py_ssize_t -iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, int cnt, int type) +iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, Py_ssize_t cnt, int type) { - int i, j; + Py_ssize_t i, j; Py_ssize_t blen, total = 0; *iov = PyMem_New(struct iovec, cnt); @@ -7965,8 +7968,7 @@ static Py_ssize_t os_readv_impl(PyObject *module, int fd, PyObject *buffers) /*[clinic end generated code: output=792da062d3fcebdb input=e679eb5dbfa0357d]*/ { - int cnt; - Py_ssize_t n; + Py_ssize_t cnt, n; int async_err = 0; struct iovec *iov; Py_buffer *buf; @@ -7978,6 +7980,8 @@ os_readv_impl(PyObject *module, int fd, PyObject *buffers) } cnt = PySequence_Size(buffers); + if (cnt < 0) + return -1; if (iov_setup(&iov, &buf, buffers, cnt, PyBUF_WRITABLE) < 0) return -1; @@ -8116,15 +8120,24 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict) "sendfile() headers must be a sequence"); return NULL; } else { - Py_ssize_t i = 0; /* Avoid uninitialized warning */ - sf.hdr_cnt = PySequence_Size(headers); - if (sf.hdr_cnt > 0 && - (i = iov_setup(&(sf.headers), &hbuf, - headers, sf.hdr_cnt, PyBUF_SIMPLE)) < 0) + Py_ssize_t i = PySequence_Size(headers); + if (i < 0) + return NULL; + if (i > INT_MAX) { + PyErr_SetString(PyExc_OverflowError, + "sendfile() header is too large"); return NULL; + } + if (i > 0) { + sf.hdr_cnt = (int)i; + i = iov_setup(&(sf.headers), &hbuf, + headers, sf.hdr_cnt, PyBUF_SIMPLE); + if (i < 0) + return NULL; #ifdef __APPLE__ - sbytes += i; + sbytes += i; #endif + } } } if (trailers != NULL) { @@ -8133,15 +8146,24 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict) "sendfile() trailers must be a sequence"); return NULL; } else { - Py_ssize_t i = 0; /* Avoid uninitialized warning */ - sf.trl_cnt = PySequence_Size(trailers); - if (sf.trl_cnt > 0 && - (i = iov_setup(&(sf.trailers), &tbuf, - trailers, sf.trl_cnt, PyBUF_SIMPLE)) < 0) + Py_ssize_t i = PySequence_Size(trailers); + if (i < 0) + return NULL; + if (i > INT_MAX) { + PyErr_SetString(PyExc_OverflowError, + "sendfile() trailer is too large"); return NULL; + } + if (i > 0) { + sf.trl_cnt = (int)i; + i = iov_setup(&(sf.trailers), &tbuf, + trailers, sf.trl_cnt, PyBUF_SIMPLE); + if (i < 0) + return NULL; #ifdef __APPLE__ - sbytes += i; + sbytes += i; #endif + } } } @@ -8411,7 +8433,7 @@ static Py_ssize_t os_writev_impl(PyObject *module, int fd, PyObject *buffers) /*[clinic end generated code: output=56565cfac3aac15b input=5b8d17fe4189d2fe]*/ { - int cnt; + Py_ssize_t cnt; Py_ssize_t result; int async_err = 0; struct iovec *iov; @@ -8423,6 +8445,8 @@ os_writev_impl(PyObject *module, int fd, PyObject *buffers) return -1; } cnt = PySequence_Size(buffers); + if (cnt < 0) + return -1; if (iov_setup(&iov, &buf, buffers, cnt, PyBUF_SIMPLE) < 0) { return -1; diff --git a/Objects/setobject.c b/Objects/setobject.c index 9fe28138c0012b..c1bc1e1234799f 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1550,20 +1550,26 @@ set_difference(PySetObject *so, PyObject *other) PyObject *key; Py_hash_t hash; setentry *entry; - Py_ssize_t pos = 0; + Py_ssize_t pos = 0, other_size; int rv; if (PySet_GET_SIZE(so) == 0) { return set_copy(so); } - if (!PyAnySet_Check(other) && !PyDict_CheckExact(other)) { + if (PyAnySet_Check(other)) { + other_size = PySet_GET_SIZE(other); + } + else if (PyDict_CheckExact(other)) { + other_size = PyDict_Size(other); + } + else { return set_copy_and_difference(so, other); } /* If len(so) much more than len(other), it's more efficient to simply copy * so and then iterate other looking for common elements. */ - if ((PySet_GET_SIZE(so) >> 2) > PyObject_Size(other)) { + if ((PySet_GET_SIZE(so) >> 2) > other_size) { return set_copy_and_difference(so, other); }