Skip to content

gh-117021: Fix integer overflow in PyLong_AsPid() on non-Windows 64-bit platforms #117064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/Python.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#include "bytearrayobject.h"
#include "bytesobject.h"
#include "unicodeobject.h"
#include "pyerrors.h"
#include "longobject.h"
#include "cpython/longintrepr.h"
#include "boolobject.h"
Expand Down Expand Up @@ -99,7 +100,6 @@
#include "cpython/picklebufobject.h"
#include "cpython/pytime.h"
#include "codecs.h"
#include "pyerrors.h"
#include "pythread.h"
#include "cpython/context.h"
#include "modsupport.h"
Expand Down
19 changes: 18 additions & 1 deletion Include/longobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,24 @@ PyAPI_FUNC(PyObject *) PyLong_GetInfo(void);
#if !defined(SIZEOF_PID_T) || SIZEOF_PID_T == SIZEOF_INT
#define _Py_PARSE_PID "i"
#define PyLong_FromPid PyLong_FromLong
#define PyLong_AsPid PyLong_AsLong
# if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000
# define PyLong_AsPid PyLong_AsInt
# elif SIZEOF_INT == SIZEOF_LONG
# define PyLong_AsPid PyLong_AsLong
# else
static inline int
PyLong_AsPid(PyObject *obj)
{
int overflow;
long result = PyLong_AsLongAndOverflow(obj, &overflow);
if (overflow || result > INT_MAX || result < INT_MIN) {
PyErr_SetString(PyExc_OverflowError,
"Python int too large to convert to C int");
return -1;
}
return (int)result;
}
# endif
#elif SIZEOF_PID_T == SIZEOF_LONG
#define _Py_PARSE_PID "l"
#define PyLong_FromPid PyLong_FromLong
Expand Down
28 changes: 28 additions & 0 deletions Lib/test/test_capi/test_long.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,34 @@ def test_long_asvoidptr(self):
self.assertRaises(OverflowError, asvoidptr, -2**1000)
# CRASHES asvoidptr(NULL)

def _test_long_aspid(self, aspid):
# Test PyLong_AsPid()
from _testcapi import SIZEOF_PID_T
bits = 8 * SIZEOF_PID_T
PID_T_MIN = -2**(bits-1)
PID_T_MAX = 2**(bits-1) - 1
# round trip (object -> long -> object)
for value in (PID_T_MIN, PID_T_MAX, -1, 0, 1, 1234):
with self.subTest(value=value):
self.assertEqual(aspid(value), value)

self.assertEqual(aspid(IntSubclass(42)), 42)
self.assertEqual(aspid(Index(42)), 42)
self.assertEqual(aspid(MyIndexAndInt()), 10)

self.assertRaises(OverflowError, aspid, PID_T_MIN - 1)
self.assertRaises(OverflowError, aspid, PID_T_MAX + 1)
self.assertRaises(TypeError, aspid, 1.0)
self.assertRaises(TypeError, aspid, b'2')
self.assertRaises(TypeError, aspid, '3')
self.assertRaises(SystemError, aspid, NULL)

def test_long_aspid(self):
self._test_long_aspid(_testcapi.pylong_aspid)

def test_long_aspid_limited(self):
self._test_long_aspid(_testlimitedcapi.pylong_aspid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, that's very interesting, testing two implementations thanks to the new _testlimitedcapi extension! That's cool!


def test_long_asnativebytes(self):
import math
from _testcapi import (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix integer overflow in :c:func:`PyLong_AsPid` on non-Windows 64-bit
platforms.
12 changes: 12 additions & 0 deletions Modules/_testcapi/long.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,24 @@ pylong_fromnativebytes(PyObject *module, PyObject *args)
return res;
}

static PyObject *
pylong_aspid(PyObject *module, PyObject *arg)
{
NULLABLE(arg);
pid_t value = PyLong_AsPid(arg);
if (value == -1 && PyErr_Occurred()) {
return NULL;
}
return PyLong_FromPid(value);
}


static PyMethodDef test_methods[] = {
_TESTCAPI_CALL_LONG_COMPACT_API_METHODDEF
{"pylong_fromunicodeobject", pylong_fromunicodeobject, METH_VARARGS},
{"pylong_asnativebytes", pylong_asnativebytes, METH_VARARGS},
{"pylong_fromnativebytes", pylong_fromnativebytes, METH_VARARGS},
{"pylong_aspid", pylong_aspid, METH_O},
{NULL},
};

Expand Down
1 change: 1 addition & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -3946,6 +3946,7 @@ PyInit__testcapi(void)
PyModule_AddObject(m, "SIZEOF_WCHAR_T", PyLong_FromSsize_t(sizeof(wchar_t)));
PyModule_AddObject(m, "SIZEOF_VOID_P", PyLong_FromSsize_t(sizeof(void*)));
PyModule_AddObject(m, "SIZEOF_TIME_T", PyLong_FromSsize_t(sizeof(time_t)));
PyModule_AddObject(m, "SIZEOF_PID_T", PyLong_FromSsize_t(sizeof(pid_t)));
PyModule_AddObject(m, "Py_Version", PyLong_FromUnsignedLong(Py_Version));
Py_INCREF(&PyInstanceMethod_Type);
PyModule_AddObject(m, "instancemethod", (PyObject *)&PyInstanceMethod_Type);
Expand Down
12 changes: 12 additions & 0 deletions Modules/_testlimitedcapi/long.c
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,17 @@ pylong_asvoidptr(PyObject *module, PyObject *arg)
return Py_NewRef((PyObject *)value);
}

static PyObject *
pylong_aspid(PyObject *module, PyObject *arg)
{
NULLABLE(arg);
pid_t value = PyLong_AsPid(arg);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is built with #define Py_LIMITED_API 0x030d0000 (defined at the top of long.c). So the limited C API 3.12 implementation is not tested.

I propose to merge the PR like that but consider later to find a way to test multiple versions of the limited C API. IMO we need at least separated .c files in Modules/_testlimitedcapi/. For example, long_36.c for limited C API 3.6?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it would be better to define Py_LIMITED_API before includes, so the same source can be used to generate code for different modules: _testcapi, _testcapi35, _testcapi313, etc. If PyLong_AsInt is only defined for the unlimited C API and the limited C API 3.13, its wrapper should be surrounded by #if/#endif.

if (value == -1 && PyErr_Occurred()) {
return NULL;
}
return PyLong_FromPid(value);
}


static PyMethodDef test_methods[] = {
_TESTLIMITEDCAPI_TEST_LONG_AND_OVERFLOW_METHODDEF
Expand Down Expand Up @@ -773,6 +784,7 @@ static PyMethodDef test_methods[] = {
{"pylong_as_size_t", pylong_as_size_t, METH_O},
{"pylong_asdouble", pylong_asdouble, METH_O},
{"pylong_asvoidptr", pylong_asvoidptr, METH_O},
{"pylong_aspid", pylong_aspid, METH_O},
{NULL},
};

Expand Down