diff options
| -rw-r--r-- | Lib/test/test_os.py | 50 | ||||
| -rw-r--r-- | Lib/test/test_posix.py | 53 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst | 3 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst | 3 | ||||
| -rw-r--r-- | Modules/posixmodule.c | 37 |
5 files changed, 114 insertions, 32 deletions
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index e509188..a140ae0 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2532,12 +2532,14 @@ class SendfileTestServer(asyncore.dispatcher, threading.Thread): def __init__(self, conn): asynchat.async_chat.__init__(self, conn) self.in_buffer = [] + self.accumulate = True self.closed = False self.push(b"220 ready\r\n") def handle_read(self): data = self.recv(4096) - self.in_buffer.append(data) + if self.accumulate: + self.in_buffer.append(data) def get_data(self): return b''.join(self.in_buffer) @@ -2618,6 +2620,8 @@ class TestSendfile(unittest.TestCase): not sys.platform.startswith("sunos") requires_headers_trailers = unittest.skipUnless(SUPPORT_HEADERS_TRAILERS, 'requires headers and trailers support') + requires_32b = unittest.skipUnless(sys.maxsize < 2**32, + 'test is only meaningful on 32-bit builds') @classmethod def setUpClass(cls): @@ -2648,17 +2652,13 @@ class TestSendfile(unittest.TestCase): self.server.stop() self.server = None - def sendfile_wrapper(self, sock, file, offset, nbytes, headers=[], trailers=[]): + def sendfile_wrapper(self, *args, **kwargs): """A higher level wrapper representing how an application is supposed to use sendfile(). """ - while 1: + while True: try: - if self.SUPPORT_HEADERS_TRAILERS: - return os.sendfile(sock, file, offset, nbytes, headers, - trailers) - else: - return os.sendfile(sock, file, offset, nbytes) + return os.sendfile(*args, **kwargs) except OSError as err: if err.errno == errno.ECONNRESET: # disconnected @@ -2749,20 +2749,22 @@ class TestSendfile(unittest.TestCase): @requires_headers_trailers def test_headers(self): total_sent = 0 + expected_data = b"x" * 512 + b"y" * 256 + self.DATA[:-1] sent = os.sendfile(self.sockno, self.fileno, 0, 4096, - headers=[b"x" * 512]) + headers=[b"x" * 512, b"y" * 256]) + self.assertLessEqual(sent, 512 + 256 + 4096) total_sent += sent offset = 4096 - nbytes = 4096 - while 1: + while total_sent < len(expected_data): + nbytes = min(len(expected_data) - total_sent, 4096) sent = self.sendfile_wrapper(self.sockno, self.fileno, offset, nbytes) if sent == 0: break + self.assertLessEqual(sent, nbytes) total_sent += sent offset += sent - expected_data = b"x" * 512 + self.DATA self.assertEqual(total_sent, len(expected_data)) self.client.close() self.server.wait() @@ -2778,12 +2780,30 @@ class TestSendfile(unittest.TestCase): create_file(TESTFN2, file_data) with open(TESTFN2, 'rb') as f: - os.sendfile(self.sockno, f.fileno(), 0, len(file_data), - trailers=[b"1234"]) + os.sendfile(self.sockno, f.fileno(), 0, 5, + trailers=[b"123456", b"789"]) self.client.close() self.server.wait() data = self.server.handler_instance.get_data() - self.assertEqual(data, b"abcdef1234") + self.assertEqual(data, b"abcde123456789") + + @requires_headers_trailers + @requires_32b + def test_headers_overflow_32bits(self): + self.server.handler_instance.accumulate = False + with self.assertRaises(OSError) as cm: + os.sendfile(self.sockno, self.fileno, 0, 0, + headers=[b"x" * 2**16] * 2**15) + self.assertEqual(cm.exception.errno, errno.EINVAL) + + @requires_headers_trailers + @requires_32b + def test_trailers_overflow_32bits(self): + self.server.handler_instance.accumulate = False + with self.assertRaises(OSError) as cm: + os.sendfile(self.sockno, self.fileno, 0, 0, + trailers=[b"x" * 2**16] * 2**15) + self.assertEqual(cm.exception.errno, errno.EINVAL) @requires_headers_trailers @unittest.skipUnless(hasattr(os, 'SF_NODISKIO'), diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 4f6abc6..e569f7a 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -20,6 +20,9 @@ import warnings _DUMMY_SYMLINK = os.path.join(tempfile.gettempdir(), support.TESTFN + '-dummy-symlink') +requires_32b = unittest.skipUnless(sys.maxsize < 2**32, + 'test is only meaningful on 32-bit builds') + class PosixTester(unittest.TestCase): def setUp(self): @@ -284,6 +287,7 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'preadv'), "test needs posix.preadv()") @unittest.skipUnless(hasattr(posix, 'RWF_HIPRI'), "test needs posix.RWF_HIPRI") def test_preadv_flags(self): fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) @@ -295,6 +299,19 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'preadv'), "test needs posix.preadv()") + @requires_32b + def test_preadv_overflow_32bits(self): + fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) + try: + buf = [bytearray(2**16)] * 2**15 + with self.assertRaises(OSError) as cm: + os.preadv(fd, buf, 0) + self.assertEqual(cm.exception.errno, errno.EINVAL) + self.assertEqual(bytes(buf[0]), b'\0'* 2**16) + finally: + os.close(fd) + @unittest.skipUnless(hasattr(posix, 'pwrite'), "test needs posix.pwrite()") def test_pwrite(self): fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) @@ -320,6 +337,7 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'pwritev'), "test needs posix.pwritev()") @unittest.skipUnless(hasattr(posix, 'os.RWF_SYNC'), "test needs os.RWF_SYNC") def test_pwritev_flags(self): fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) @@ -334,6 +352,17 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'pwritev'), "test needs posix.pwritev()") + @requires_32b + def test_pwritev_overflow_32bits(self): + fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) + try: + with self.assertRaises(OSError) as cm: + os.pwritev(fd, [b"x" * 2**16] * 2**15, 0) + self.assertEqual(cm.exception.errno, errno.EINVAL) + finally: + os.close(fd) + @unittest.skipUnless(hasattr(posix, 'posix_fallocate'), "test needs posix.posix_fallocate()") def test_posix_fallocate(self): @@ -435,6 +464,17 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'writev'), "test needs posix.writev()") + @requires_32b + def test_writev_overflow_32bits(self): + fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) + try: + with self.assertRaises(OSError) as cm: + os.writev(fd, [b"x" * 2**16] * 2**15) + self.assertEqual(cm.exception.errno, errno.EINVAL) + finally: + os.close(fd) + @unittest.skipUnless(hasattr(posix, 'readv'), "test needs posix.readv()") def test_readv(self): fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) @@ -457,6 +497,19 @@ class PosixTester(unittest.TestCase): finally: os.close(fd) + @unittest.skipUnless(hasattr(posix, 'readv'), "test needs posix.readv()") + @requires_32b + def test_readv_overflow_32bits(self): + fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT) + try: + buf = [bytearray(2**16)] * 2**15 + with self.assertRaises(OSError) as cm: + os.readv(fd, buf) + self.assertEqual(cm.exception.errno, errno.EINVAL) + self.assertEqual(bytes(buf[0]), b'\0'* 2**16) + finally: + os.close(fd) + @unittest.skipUnless(hasattr(posix, 'dup'), 'test needs posix.dup()') def test_dup(self): diff --git a/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst b/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst new file mode 100644 index 0000000..9fd1535 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst @@ -0,0 +1,3 @@ +Fixed integer overflow in :func:`os.readv`, :func:`os.writev`, +:func:`os.preadv` and :func:`os.pwritev` and in :func:`os.sendfile` with +*headers* or *trailers* arguments (on BSD-based OSes and macOS). diff --git a/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst b/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst new file mode 100644 index 0000000..547342c --- /dev/null +++ b/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst @@ -0,0 +1,3 @@ +Fixed sending the part of the file in :func:`os.sendfile` on macOS. Using +the *trailers* argument could cause sending more bytes from the input file +than was specified. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 435e5f6..6104c0f 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8321,12 +8321,13 @@ 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 + || defined(__APPLE__))) \ + || defined(HAVE_READV) || defined(HAVE_PREADV) || defined (HAVE_PREADV2) \ + || defined(HAVE_WRITEV) || defined(HAVE_PWRITEV) || defined (HAVE_PWRITEV2) +static int iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, Py_ssize_t cnt, int type) { Py_ssize_t i, j; - Py_ssize_t blen, total = 0; *iov = PyMem_New(struct iovec, cnt); if (*iov == NULL) { @@ -8351,11 +8352,9 @@ iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, Py_ssize_t cnt, in } Py_DECREF(item); (*iov)[i].iov_base = (*buf)[i].buf; - blen = (*buf)[i].len; - (*iov)[i].iov_len = blen; - total += blen; + (*iov)[i].iov_len = (*buf)[i].len; } - return total; + return 0; fail: PyMem_Del(*iov); @@ -8652,12 +8651,20 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict) } if (i > 0) { sf.hdr_cnt = (int)i; - i = iov_setup(&(sf.headers), &hbuf, - headers, sf.hdr_cnt, PyBUF_SIMPLE); - if (i < 0) + if (iov_setup(&(sf.headers), &hbuf, + headers, sf.hdr_cnt, PyBUF_SIMPLE) < 0) return NULL; #ifdef __APPLE__ - sbytes += i; + for (i = 0; i < sf.hdr_cnt; i++) { + Py_ssize_t blen = sf.headers[i].iov_len; +# define OFF_T_MAX 0x7fffffffffffffff + if (sbytes >= OFF_T_MAX - blen) { + PyErr_SetString(PyExc_OverflowError, + "sendfile() header is too large"); + return NULL; + } + sbytes += blen; + } #endif } } @@ -8678,13 +8685,9 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict) } if (i > 0) { sf.trl_cnt = (int)i; - i = iov_setup(&(sf.trailers), &tbuf, - trailers, sf.trl_cnt, PyBUF_SIMPLE); - if (i < 0) + if (iov_setup(&(sf.trailers), &tbuf, + trailers, sf.trl_cnt, PyBUF_SIMPLE) < 0) return NULL; -#ifdef __APPLE__ - sbytes += i; -#endif } } } |