From 78980438683d98076cd541d995a868fb5c9e4277 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 15 Jan 2013 01:12:17 +0200 Subject: Issue #15989: Fix several occurrences of integer overflow when result of PyLong_AsLong() narrowed to int without checks. --- Include/longobject.h | 3 +++ Lib/ctypes/test/test_structures.py | 9 +++++++++ Lib/test/string_tests.py | 11 +++++++++++ Lib/test/test_fcntl.py | 21 +++++++++++++++++++++ Lib/test/test_fileio.py | 4 ++++ Lib/test/test_io.py | 9 +++++++++ Lib/test/test_poll.py | 10 ++++++++++ Lib/test/test_posix.py | 5 +++++ Lib/test/test_socket.py | 22 +++++++++++++++++++--- Modules/_ctypes/stgdict.c | 2 +- Modules/_io/fileio.c | 4 ++-- Modules/_io/textio.c | 2 +- Modules/parsermodule.c | 24 +++++++++++++++++++----- Modules/posixmodule.c | 2 +- Modules/selectmodule.c | 12 ++++++++---- Modules/socketmodule.c | 6 +++--- Objects/fileobject.c | 4 ++-- Objects/longobject.c | 18 ++++++++++++++++++ Objects/unicodeobject.c | 4 ++-- 19 files changed, 148 insertions(+), 24 deletions(-) diff --git a/Include/longobject.h b/Include/longobject.h index d741f1b..cd0cf30 100644 --- a/Include/longobject.h +++ b/Include/longobject.h @@ -26,6 +26,9 @@ PyAPI_FUNC(Py_ssize_t) PyLong_AsSsize_t(PyObject *); PyAPI_FUNC(size_t) PyLong_AsSize_t(PyObject *); PyAPI_FUNC(unsigned long) PyLong_AsUnsignedLong(PyObject *); PyAPI_FUNC(unsigned long) PyLong_AsUnsignedLongMask(PyObject *); +#ifndef Py_LIMITED_API +PyAPI_FUNC(int) _PyLong_AsInt(PyObject *); +#endif PyAPI_FUNC(PyObject *) PyLong_GetInfo(void); /* It may be useful in the future. I've added it in the PyInt -> PyLong diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index b6d8b01..d764ce2 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -1,6 +1,7 @@ import unittest from ctypes import * from struct import calcsize +import _testcapi class SubclassesTest(unittest.TestCase): def test_subclass(self): @@ -199,6 +200,14 @@ class StructureTestCase(unittest.TestCase): "_pack_": -1} self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) + # Issue 15989 + d = {"_fields_": [("a", c_byte)], + "_pack_": _testcapi.INT_MAX + 1} + self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) + d = {"_fields_": [("a", c_byte)], + "_pack_": _testcapi.UINT_MAX + 2} + self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) + def test_initializers(self): class Person(Structure): _fields_ = [("name", c_char*6), diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 27e4662..385b039 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -5,6 +5,7 @@ Common tests shared by test_str, test_unicode, test_userstring and test_string. import unittest, string, sys, struct from test import support from collections import UserList +import _testcapi class Sequence: def __init__(self, seq='wxyz'): self.seq = seq @@ -1206,6 +1207,16 @@ class MixinStrUnicodeUserStringTest: self.checkraises(ValueError, '%%%df' % (2**64), '__mod__', (3.2)) self.checkraises(ValueError, '%%.%df' % (2**64), '__mod__', (3.2)) + self.checkraises(OverflowError, '%*s', '__mod__', + (_testcapi.PY_SSIZE_T_MAX + 1, '')) + self.checkraises(OverflowError, '%.*f', '__mod__', + (_testcapi.INT_MAX + 1, 1. / 7)) + # Issue 15989 + self.checkraises(OverflowError, '%*s', '__mod__', + (1 << (_testcapi.PY_SSIZE_T_MAX.bit_length() + 1), '')) + self.checkraises(OverflowError, '%.*f', '__mod__', + (_testcapi.UINT_MAX + 1, 1. / 7)) + class X(object): pass self.checkraises(TypeError, 'abc', '__mod__', X()) diff --git a/Lib/test/test_fcntl.py b/Lib/test/test_fcntl.py index 6d9ee0b..f977187 100644 --- a/Lib/test/test_fcntl.py +++ b/Lib/test/test_fcntl.py @@ -3,6 +3,7 @@ import os import struct import sys +import _testcapi import unittest from test.support import verbose, TESTFN, unlink, run_unittest, import_module @@ -69,6 +70,26 @@ class TestFcntl(unittest.TestCase): rv = fcntl.fcntl(self.f, fcntl.F_SETLKW, lockdata) self.f.close() + def test_fcntl_bad_file(self): + class F: + def __init__(self, fn): + self.fn = fn + def fileno(self): + return self.fn + self.assertRaises(ValueError, fcntl.fcntl, -1, fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(ValueError, fcntl.fcntl, F(-1), fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(TypeError, fcntl.fcntl, 'spam', fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(TypeError, fcntl.fcntl, F('spam'), fcntl.F_SETFL, os.O_NONBLOCK) + # Issue 15989 + self.assertRaises(OverflowError, fcntl.fcntl, _testcapi.INT_MAX + 1, + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(OverflowError, fcntl.fcntl, F(_testcapi.INT_MAX + 1), + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(OverflowError, fcntl.fcntl, _testcapi.INT_MIN - 1, + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(OverflowError, fcntl.fcntl, F(_testcapi.INT_MIN - 1), + fcntl.F_SETFL, os.O_NONBLOCK) + def test_fcntl_64_bit(self): # Issue #1309352: fcntl shouldn't fail when the third arg fits in a # C 'long' but not in a C 'int'. diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index 0badf51..0ccbda2 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -8,6 +8,7 @@ import unittest from array import array from weakref import proxy from functools import wraps +import _testcapi from test.support import TESTFN, check_warnings, run_unittest, make_bad_fd from collections import UserList @@ -347,6 +348,9 @@ class OtherFileTests(unittest.TestCase): if sys.platform == 'win32': import msvcrt self.assertRaises(OSError, msvcrt.get_osfhandle, make_bad_fd()) + # Issue 15989 + self.assertRaises(TypeError, _FileIO, _testcapi.INT_MAX + 1) + self.assertRaises(TypeError, _FileIO, _testcapi.INT_MIN - 1) def testBadModeArgument(self): # verify that we get a sensible error message for bad mode argument diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 7906060..8727dde 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -32,6 +32,7 @@ import time import unittest import warnings import weakref +import _testcapi from collections import deque, UserList from itertools import cycle, count from test import support @@ -1970,6 +1971,14 @@ class TextIOWrapperTest(unittest.TestCase): os.environ.clear() os.environ.update(old_environ) + # Issue 15989 + def test_device_encoding(self): + b = self.BytesIO() + b.fileno = lambda: _testcapi.INT_MAX + 1 + self.assertRaises(OverflowError, self.TextIOWrapper, b) + b.fileno = lambda: _testcapi.UINT_MAX + 1 + self.assertRaises(OverflowError, self.TextIOWrapper, b) + def test_encoding(self): # Check the encoding attribute is always set, and valid b = self.BytesIO() diff --git a/Lib/test/test_poll.py b/Lib/test/test_poll.py index 3b7926d..a2fec3d 100644 --- a/Lib/test/test_poll.py +++ b/Lib/test/test_poll.py @@ -1,6 +1,7 @@ # Test case for the os.poll() function import os, select, random, unittest, subprocess +import _testcapi from test.support import TESTFN, run_unittest try: @@ -151,6 +152,15 @@ class PollTests(unittest.TestCase): if x != 5: self.fail('Overflow must have occurred') + pollster = select.poll() + # Issue 15989 + self.assertRaises(OverflowError, pollster.register, 0, + _testcapi.SHRT_MAX + 1) + self.assertRaises(OverflowError, pollster.register, 0, + _testcapi.USHRT_MAX + 1) + self.assertRaises(OverflowError, pollster.poll, _testcapi.INT_MAX + 1) + self.assertRaises(OverflowError, pollster.poll, _testcapi.UINT_MAX + 1) + def test_main(): run_unittest(PollTests) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 1352943..9789830 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -17,6 +17,7 @@ import stat import tempfile import unittest import warnings +import _testcapi _DUMMY_SYMLINK = os.path.join(tempfile.gettempdir(), support.TESTFN + '-dummy-symlink') @@ -537,6 +538,10 @@ class PosixTester(unittest.TestCase): except OSError: pass + # Issue 15989 + self.assertRaises(OverflowError, os.pipe2, _testcapi.INT_MAX + 1) + self.assertRaises(OverflowError, os.pipe2, _testcapi.UINT_MAX + 1) + def test_utime(self): if hasattr(posix, 'utime'): now = time.time() diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index be9206e..282596f 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1262,11 +1262,17 @@ class GeneralModuleTests(unittest.TestCase): for protocol in range(pickle.HIGHEST_PROTOCOL + 1): self.assertRaises(TypeError, pickle.dumps, sock, protocol) - def test_listen_backlog0(self): + def test_listen_backlog(self): + for backlog in 0, -1: + srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + srv.bind((HOST, 0)) + srv.listen(backlog) + srv.close() + + # Issue 15989 srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) srv.bind((HOST, 0)) - # backlog = 0 - srv.listen(0) + self.assertRaises(OverflowError, srv.listen, _testcapi.INT_MAX + 1) srv.close() @unittest.skipUnless(support.IPV6_ENABLED, 'IPv6 required for this test.') @@ -1582,6 +1588,11 @@ class BasicTCPTest(SocketConnectedTest): def _testShutdown(self): self.serv_conn.send(MSG) + # Issue 15989 + self.assertRaises(OverflowError, self.serv_conn.shutdown, + _testcapi.INT_MAX + 1) + self.assertRaises(OverflowError, self.serv_conn.shutdown, + 2 + (_testcapi.UINT_MAX + 1)) self.serv_conn.shutdown(2) def testDetach(self): @@ -3563,6 +3574,11 @@ class NonBlockingTCPTests(ThreadedTCPSocketTest): pass end = time.time() self.assertTrue((end - start) < 1.0, "Error setting non-blocking mode.") + # Issue 15989 + self.assertRaises(OverflowError, self.serv.setblocking, + _testcapi.INT_MAX + 1) + self.assertRaises(OverflowError, self.serv.setblocking, + _testcapi.UINT_MAX + 1) def _testSetBlocking(self): pass diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index e5b0e4c..25d9996 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -335,7 +335,7 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct isPacked = PyObject_GetAttrString(type, "_pack_"); if (isPacked) { - pack = PyLong_AsLong(isPacked); + pack = _PyLong_AsInt(isPacked); if (pack < 0 || PyErr_Occurred()) { Py_XDECREF(isPacked); PyErr_SetString(PyExc_ValueError, diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 34425b7..fd7c1fc 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -244,7 +244,7 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds) return -1; } - fd = PyLong_AsLong(nameobj); + fd = _PyLong_AsInt(nameobj); if (fd < 0) { if (!PyErr_Occurred()) { PyErr_SetString(PyExc_ValueError, @@ -382,7 +382,7 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds) goto error; } - self->fd = PyLong_AsLong(fdobj); + self->fd = _PyLong_AsInt(fdobj); Py_DECREF(fdobj); if (self->fd == -1) { goto error; diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 4fc0caa..afd8d41 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -881,7 +881,7 @@ textiowrapper_init(textio *self, PyObject *args, PyObject *kwds) } } else { - int fd = (int) PyLong_AsLong(fileno); + int fd = _PyLong_AsInt(fileno); Py_DECREF(fileno); if (fd == -1 && PyErr_Occurred()) { goto error; diff --git a/Modules/parsermodule.c b/Modules/parsermodule.c index fea603e..e86fe4d 100644 --- a/Modules/parsermodule.c +++ b/Modules/parsermodule.c @@ -725,7 +725,7 @@ build_node_children(PyObject *tuple, node *root, int *line_num) /* elem must always be a sequence, however simple */ PyObject* elem = PySequence_GetItem(tuple, i); int ok = elem != NULL; - long type = 0; + int type = 0; char *strn = 0; if (ok) @@ -736,8 +736,14 @@ build_node_children(PyObject *tuple, node *root, int *line_num) ok = 0; else { ok = PyLong_Check(temp); - if (ok) - type = PyLong_AS_LONG(temp); + if (ok) { + type = _PyLong_AsInt(temp); + if (type == -1 && PyErr_Occurred()) { + Py_DECREF(temp); + Py_DECREF(elem); + return 0; + } + } Py_DECREF(temp); } } @@ -773,8 +779,16 @@ build_node_children(PyObject *tuple, node *root, int *line_num) if (len == 3) { PyObject *o = PySequence_GetItem(elem, 2); if (o != NULL) { - if (PyLong_Check(o)) - *line_num = PyLong_AS_LONG(o); + if (PyLong_Check(o)) { + int num = _PyLong_AsInt(o); + if (num == -1 && PyErr_Occurred()) { + Py_DECREF(o); + Py_DECREF(temp); + Py_DECREF(elem); + return 0; + } + *line_num = num; + } else { PyErr_Format(parser_error, "third item in terminal node must be an" diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 24585c5..fb58507 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7647,7 +7647,7 @@ posix_pipe2(PyObject *self, PyObject *arg) int fds[2]; int res; - flags = PyLong_AsLong(arg); + flags = _PyLong_AsInt(arg); if (flags == -1 && PyErr_Occurred()) return NULL; diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index 52be4d8..e79bea3 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -352,10 +352,13 @@ update_ufd_array(pollObject *self) i = pos = 0; while (PyDict_Next(self->dict, &pos, &key, &value)) { - self->ufds[i].fd = PyLong_AsLong(key); + assert(i < self->ufd_len); + /* Never overflow */ + self->ufds[i].fd = (int)PyLong_AsLong(key); self->ufds[i].events = (short)PyLong_AsLong(value); i++; } + assert(i == self->ufd_len); self->ufd_uptodate = 1; return 1; } @@ -371,10 +374,11 @@ static PyObject * poll_register(pollObject *self, PyObject *args) { PyObject *o, *key, *value; - int fd, events = POLLIN | POLLPRI | POLLOUT; + int fd; + short events = POLLIN | POLLPRI | POLLOUT; int err; - if (!PyArg_ParseTuple(args, "O|i:register", &o, &events)) { + if (!PyArg_ParseTuple(args, "O|h:register", &o, &events)) { return NULL; } @@ -513,7 +517,7 @@ poll_poll(pollObject *self, PyObject *args) tout = PyNumber_Long(tout); if (!tout) return NULL; - timeout = PyLong_AsLong(tout); + timeout = _PyLong_AsInt(tout); Py_DECREF(tout); if (timeout == -1 && PyErr_Occurred()) return NULL; diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 4cd6903..94793c9 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2013,7 +2013,7 @@ For IP sockets, the address info is a pair (hostaddr, port)."); static PyObject * sock_setblocking(PySocketSockObject *s, PyObject *arg) { - int block; + long block; block = PyLong_AsLong(arg); if (block == -1 && PyErr_Occurred()) @@ -2495,7 +2495,7 @@ sock_listen(PySocketSockObject *s, PyObject *arg) int backlog; int res; - backlog = PyLong_AsLong(arg); + backlog = _PyLong_AsInt(arg); if (backlog == -1 && PyErr_Occurred()) return NULL; Py_BEGIN_ALLOW_THREADS @@ -3647,7 +3647,7 @@ sock_shutdown(PySocketSockObject *s, PyObject *arg) int how; int res; - how = PyLong_AsLong(arg); + how = _PyLong_AsInt(arg); if (how == -1 && PyErr_Occurred()) return NULL; Py_BEGIN_ALLOW_THREADS diff --git a/Objects/fileobject.c b/Objects/fileobject.c index e1c47ce..3a31314 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -200,7 +200,7 @@ PyObject_AsFileDescriptor(PyObject *o) _Py_IDENTIFIER(fileno); if (PyLong_Check(o)) { - fd = PyLong_AsLong(o); + fd = _PyLong_AsInt(o); } else if ((meth = _PyObject_GetAttrId(o, &PyId_fileno)) != NULL) { @@ -210,7 +210,7 @@ PyObject_AsFileDescriptor(PyObject *o) return -1; if (PyLong_Check(fno)) { - fd = PyLong_AsLong(fno); + fd = _PyLong_AsInt(fno); Py_DECREF(fno); } else { diff --git a/Objects/longobject.c b/Objects/longobject.c index 5a50f24..1a82b1c6 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -434,6 +434,24 @@ PyLong_AsLong(PyObject *obj) return result; } +/* Get a C int from a long int object or any object that has an __int__ + method. Return -1 and set an error if overflow occurs. */ + +int +_PyLong_AsInt(PyObject *obj) +{ + int overflow; + long result = PyLong_AsLongAndOverflow(obj, &overflow); + if (overflow || result > INT_MAX || result < INT_MIN) { + /* XXX: could be cute and give a different + message for overflow == -1 */ + PyErr_SetString(PyExc_OverflowError, + "Python int too large to convert to C int"); + return -1; + } + return (int)result; +} + /* Get a Py_ssize_t from a long int object. Returns -1 and sets an error condition if overflow occurs. */ diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 6a12d71..65393d2 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -13521,7 +13521,7 @@ unicode_format_arg_parse(struct unicode_formatter_t *ctx, "* wants int"); return -1; } - arg->width = PyLong_AsLong(v); + arg->width = PyLong_AsSsize_t(v); if (arg->width == -1 && PyErr_Occurred()) return -1; if (arg->width < 0) { @@ -13568,7 +13568,7 @@ unicode_format_arg_parse(struct unicode_formatter_t *ctx, "* wants int"); return -1; } - arg->prec = PyLong_AsLong(v); + arg->prec = _PyLong_AsInt(v); if (arg->prec == -1 && PyErr_Occurred()) return -1; if (arg->prec < 0) -- cgit v0.12