From 74f49ab28b91d3c23524356230feb2724ee9b23f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 19 Jan 2013 12:55:39 +0200 Subject: [PATCH] Issue #15989: Fix several occurrences of integer overflow when result of PyInt_AsLong() or PyLong_AsLong() narrowed to int without checks. This is a backport of changesets 13e2e44db99d and 525407d89277. --- Include/intobject.h | 1 + Include/longobject.h | 1 + Lib/ctypes/test/test_structures.py | 9 +++++++++ Lib/test/string_tests.py | 15 +++++++++++++++ Lib/test/test_fcntl.py | 21 +++++++++++++++++++++ Lib/test/test_fileio.py | 4 ++++ Lib/test/test_poll.py | 10 ++++++++++ Lib/test/test_socket.py | 27 +++++++++++++++++++++++---- Modules/_ctypes/stgdict.c | 2 +- Modules/_io/_iomodule.c | 7 ++++--- Modules/_io/fileio.c | 2 +- Modules/selectmodule.c | 12 ++++++++---- Modules/socketmodule.c | 6 +++--- Objects/fileobject.c | 8 ++++---- Objects/intobject.c | 14 ++++++++++++++ Objects/longobject.c | 18 ++++++++++++++++++ Objects/unicodeobject.c | 8 ++++++-- 17 files changed, 143 insertions(+), 22 deletions(-) diff --git a/Include/intobject.h b/Include/intobject.h index 78746a63484..252eea9fd92 100644 --- a/Include/intobject.h +++ b/Include/intobject.h @@ -40,6 +40,7 @@ PyAPI_FUNC(PyObject *) PyInt_FromSize_t(size_t); PyAPI_FUNC(PyObject *) PyInt_FromSsize_t(Py_ssize_t); PyAPI_FUNC(long) PyInt_AsLong(PyObject *); PyAPI_FUNC(Py_ssize_t) PyInt_AsSsize_t(PyObject *); +PyAPI_FUNC(int) _PyInt_AsInt(PyObject *); PyAPI_FUNC(unsigned long) PyInt_AsUnsignedLongMask(PyObject *); #ifdef HAVE_LONG_LONG PyAPI_FUNC(unsigned PY_LONG_LONG) PyInt_AsUnsignedLongLongMask(PyObject *); diff --git a/Include/longobject.h b/Include/longobject.h index 2b4046128ab..8be2345a46c 100644 --- a/Include/longobject.h +++ b/Include/longobject.h @@ -25,6 +25,7 @@ PyAPI_FUNC(long) PyLong_AsLongAndOverflow(PyObject *, int *); PyAPI_FUNC(unsigned long) PyLong_AsUnsignedLong(PyObject *); PyAPI_FUNC(unsigned long) PyLong_AsUnsignedLongMask(PyObject *); PyAPI_FUNC(Py_ssize_t) PyLong_AsSsize_t(PyObject *); +PyAPI_FUNC(int) _PyLong_AsInt(PyObject *); PyAPI_FUNC(PyObject *) PyLong_GetInfo(void); /* For use by intobject.c only */ diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 1bde101d7cf..2d7c816b241 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 a1615126349..f73d0ee1880 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 test_support from UserList import UserList +import _testcapi class Sequence: def __init__(self, seq='wxyz'): self.seq = seq @@ -1113,6 +1114,20 @@ class MixinStrUnicodeUserStringTest: self.checkraises(TypeError, '%10.*f', '__mod__', ('foo', 42.)) self.checkraises(ValueError, '%10', '__mod__', (42,)) + if _testcapi.PY_SSIZE_T_MAX < sys.maxint: + self.checkraises(OverflowError, '%*s', '__mod__', + (_testcapi.PY_SSIZE_T_MAX + 1, '')) + if _testcapi.INT_MAX < sys.maxint: + self.checkraises(OverflowError, '%.*f', '__mod__', + (_testcapi.INT_MAX + 1, 1. / 7)) + # Issue 15989 + if 1 << (_testcapi.PY_SSIZE_T_MAX.bit_length() + 1) <= sys.maxint: + self.checkraises(OverflowError, '%*s', '__mod__', + (1 << (_testcapi.PY_SSIZE_T_MAX.bit_length() + 1), '')) + if _testcapi.UINT_MAX < sys.maxint: + 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 df0939151f0..7a35f61396d 100644 --- a/Lib/test/test_fcntl.py +++ b/Lib/test/test_fcntl.py @@ -6,6 +6,7 @@ OS/2+EMX doesn't support the file locking operations. import os import struct import sys +import _testcapi import unittest from test.test_support import (verbose, TESTFN, unlink, run_unittest, import_module) @@ -81,6 +82,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(ValueError, fcntl.fcntl, _testcapi.INT_MAX + 1, + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(ValueError, fcntl.fcntl, F(_testcapi.INT_MAX + 1), + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(ValueError, fcntl.fcntl, _testcapi.INT_MIN - 1, + fcntl.F_SETFL, os.O_NONBLOCK) + self.assertRaises(ValueError, 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 71ec44c31e7..fb83255190f 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -10,6 +10,7 @@ from array import array from weakref import proxy from functools import wraps from UserList import UserList +import _testcapi from test.test_support import TESTFN, check_warnings, run_unittest, make_bad_fd from test.test_support import py3k_bytes as bytes @@ -343,6 +344,9 @@ class OtherFileTests(unittest.TestCase): if sys.platform == 'win32': import msvcrt self.assertRaises(IOError, 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_poll.py b/Lib/test/test_poll.py index d33af911e80..55294f8c229 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 +import _testcapi from test.test_support import TESTFN, run_unittest try: @@ -150,6 +151,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_socket.py b/Lib/test/test_socket.py index fec62efe038..05d044e37f0 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -6,6 +6,7 @@ from test import test_support import errno import socket import select +import _testcapi import time import traceback import Queue @@ -700,11 +701,17 @@ class GeneralModuleTests(unittest.TestCase): def test_sendall_interrupted_with_timeout(self): self.check_sendall_interrupted(True) - def testListenBacklog0(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(SUPPORTS_IPV6, 'IPv6 required for this test.') @@ -808,6 +815,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) @unittest.skipUnless(thread, 'Threading required for this test.') @@ -883,7 +895,10 @@ class NonBlockingTCPTests(ThreadedTCPSocketTest): def testSetBlocking(self): # Testing whether set blocking works - self.serv.setblocking(0) + self.serv.setblocking(True) + self.assertIsNone(self.serv.gettimeout()) + self.serv.setblocking(False) + self.assertEqual(self.serv.gettimeout(), 0.0) start = time.time() try: self.serv.accept() @@ -891,6 +906,10 @@ class NonBlockingTCPTests(ThreadedTCPSocketTest): pass end = time.time() self.assertTrue((end - start) < 1.0, "Error setting non-blocking mode.") + # Issue 15989 + if _testcapi.UINT_MAX < _testcapi.ULONG_MAX: + self.serv.setblocking(_testcapi.UINT_MAX + 1) + self.assertIsNone(self.serv.gettimeout()) def _testSetBlocking(self): pass diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 773233fe6ff..658e15f1aa5 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -343,7 +343,7 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct isPacked = PyObject_GetAttrString(type, "_pack_"); if (isPacked) { - pack = PyInt_AsLong(isPacked); + pack = _PyInt_AsInt(isPacked); if (pack < 0 || PyErr_Occurred()) { Py_XDECREF(isPacked); PyErr_SetString(PyExc_ValueError, diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index d73dff2746f..a024d86c168 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -300,7 +300,8 @@ io_open(PyObject *self, PyObject *args, PyObject *kwds) int text = 0, binary = 0, universal = 0; char rawmode[5], *m; - int line_buffering, isatty; + int line_buffering; + long isatty; PyObject *raw, *modeobj = NULL, *buffer = NULL, *wrapper = NULL; @@ -443,12 +444,12 @@ io_open(PyObject *self, PyObject *args, PyObject *kwds) #ifdef HAVE_STRUCT_STAT_ST_BLKSIZE { struct stat st; - long fileno; + int fileno; PyObject *res = PyObject_CallMethod(raw, "fileno", NULL); if (res == NULL) goto error; - fileno = PyInt_AsLong(res); + fileno = _PyInt_AsInt(res); Py_DECREF(res); if (fileno == -1 && PyErr_Occurred()) goto error; diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 7fe245491bb..6cd7d817c8d 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -211,7 +211,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, diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index b95b2cedc17..61e101eb7dd 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -343,10 +343,13 @@ update_ufd_array(pollObject *self) i = pos = 0; while (PyDict_Next(self->dict, &pos, &key, &value)) { - self->ufds[i].fd = PyInt_AsLong(key); + assert(i < self->ufd_len); + /* Never overflow */ + self->ufds[i].fd = (int)PyInt_AsLong(key); self->ufds[i].events = (short)PyInt_AsLong(value); i++; } + assert(i == self->ufd_len); self->ufd_uptodate = 1; return 1; } @@ -362,10 +365,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; } @@ -503,7 +507,7 @@ poll_poll(pollObject *self, PyObject *args) tout = PyNumber_Int(tout); if (!tout) return NULL; - timeout = PyInt_AsLong(tout); + timeout = _PyInt_AsInt(tout); Py_DECREF(tout); if (timeout == -1 && PyErr_Occurred()) return NULL; diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 506a2d320b6..1eaa302e7f3 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -1713,7 +1713,7 @@ info is a pair (hostaddr, port)."); static PyObject * sock_setblocking(PySocketSockObject *s, PyObject *arg) { - int block; + long block; block = PyInt_AsLong(arg); if (block == -1 && PyErr_Occurred()) @@ -2243,7 +2243,7 @@ sock_listen(PySocketSockObject *s, PyObject *arg) int backlog; int res; - backlog = PyInt_AsLong(arg); + backlog = _PyInt_AsInt(arg); if (backlog == -1 && PyErr_Occurred()) return NULL; Py_BEGIN_ALLOW_THREADS @@ -2894,7 +2894,7 @@ sock_shutdown(PySocketSockObject *s, PyObject *arg) int how; int res; - how = PyInt_AsLong(arg); + how = _PyInt_AsInt(arg); if (how == -1 && PyErr_Occurred()) return NULL; Py_BEGIN_ALLOW_THREADS diff --git a/Objects/fileobject.c b/Objects/fileobject.c index ece23703d4c..76cdf747770 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -2659,10 +2659,10 @@ int PyObject_AsFileDescriptor(PyObject *o) PyObject *meth; if (PyInt_Check(o)) { - fd = PyInt_AsLong(o); + fd = _PyInt_AsInt(o); } else if (PyLong_Check(o)) { - fd = PyLong_AsLong(o); + fd = _PyLong_AsInt(o); } else if ((meth = PyObject_GetAttrString(o, "fileno")) != NULL) { @@ -2672,11 +2672,11 @@ int PyObject_AsFileDescriptor(PyObject *o) return -1; if (PyInt_Check(fno)) { - fd = PyInt_AsLong(fno); + fd = _PyInt_AsInt(fno); Py_DECREF(fno); } else if (PyLong_Check(fno)) { - fd = PyLong_AsLong(fno); + fd = _PyLong_AsInt(fno); Py_DECREF(fno); } else { diff --git a/Objects/intobject.c b/Objects/intobject.c index 1ae8006c282..28182f93db1 100644 --- a/Objects/intobject.c +++ b/Objects/intobject.c @@ -189,6 +189,20 @@ PyInt_AsLong(register PyObject *op) return val; } +int +_PyInt_AsInt(PyObject *obj) +{ + long result = PyInt_AsLong(obj); + if (result == -1 && PyErr_Occurred()) + return -1; + if (result > INT_MAX || result < INT_MIN) { + PyErr_SetString(PyExc_OverflowError, + "Python int too large to convert to C int"); + return -1; + } + return (int)result; +} + Py_ssize_t PyInt_AsSsize_t(register PyObject *op) { diff --git a/Objects/longobject.c b/Objects/longobject.c index 5a6338f6302..fb740dc4d6c 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -339,6 +339,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 79e1b600e32..46bfe2b54c3 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -8411,7 +8411,9 @@ PyObject *PyUnicode_Format(PyObject *format, "* wants int"); goto onError; } - width = PyInt_AsLong(v); + width = PyInt_AsSsize_t(v); + if (width == -1 && PyErr_Occurred()) + goto onError; if (width < 0) { flags |= F_LJUST; width = -width; @@ -8446,7 +8448,9 @@ PyObject *PyUnicode_Format(PyObject *format, "* wants int"); goto onError; } - prec = PyInt_AsLong(v); + prec = _PyInt_AsInt(v); + if (prec == -1 && PyErr_Occurred()) + goto onError; if (prec < 0) prec = 0; if (--fmtcnt >= 0)