From a09915bc6f0859577c1861f2b9c1772a4f4dd4aa Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Wed, 22 May 2019 10:29:59 -0600 Subject: [PATCH 1/9] bpo-37013: Fix the error handling in socket.if_indextoname() --- Modules/socketmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index f376513fead1b8..ef54f91d38f235 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -6852,8 +6852,9 @@ socket_if_indextoname(PyObject *self, PyObject *arg) char name[IF_NAMESIZE + 1]; index = PyLong_AsUnsignedLong(arg); - if (index == (unsigned long) -1) + if (index == (unsigned long) -1 && PyErr_Occurred()) { return NULL; + } if (if_indextoname(index, name) == NULL) { PyErr_SetFromErrno(PyExc_OSError); From 0c8fcced3045a39c1203b044c0ad8e1adbf0a912 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Thu, 7 May 2020 09:15:32 -0600 Subject: [PATCH 2/9] Address comments. --- Lib/test/test_socket.py | 5 +++++ Modules/socketmodule.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 1aaa9e44f90c65..191d6990c8250f 100755 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1073,6 +1073,11 @@ def testInterfaceNameIndex(self): def testInvalidInterfaceIndexToName(self): self.assertRaises(OSError, socket.if_indextoname, 0) self.assertRaises(TypeError, socket.if_indextoname, '_DEADBEEF') + for i in 2**32-1, 2**32, 2**64-1, 2**64: + try: + socket.if_indextoname(i) + except (OSError, OverflowError): + pass @unittest.skipUnless(hasattr(socket, 'if_nametoindex'), 'socket.if_nametoindex() not available.') diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index ef54f91d38f235..68560b182b124c 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -6856,6 +6856,11 @@ socket_if_indextoname(PyObject *self, PyObject *arg) return NULL; } + if ((unsigned long)(unsigned int)index != index) { + PyErr_SetString(PyExc_OverflowError, "index is too large"); + return NULL; + } + if (if_indextoname(index, name) == NULL) { PyErr_SetFromErrno(PyExc_OSError); return NULL; From e69a845fdb52c7bbff827f18efcab30fbed27489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Thu, 19 May 2022 16:48:38 +0200 Subject: [PATCH 3/9] Redo the change per code review comments --- Modules/socketmodule.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 68560b182b124c..0b58dc9c5a8039 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -6845,18 +6845,22 @@ static PyObject * socket_if_indextoname(PyObject *self, PyObject *arg) { #ifdef MS_WINDOWS + NET_IFINDEX index_long; NET_IFINDEX index; #else - unsigned long index; + unsigned long index_long; + unsigned int index; #endif char name[IF_NAMESIZE + 1]; - index = PyLong_AsUnsignedLong(arg); - if (index == (unsigned long) -1 && PyErr_Occurred()) { + index_long = PyLong_AsUnsignedLong(arg); + if (index_long == (unsigned long) -1 && PyErr_Occurred()) { return NULL; } - if ((unsigned long)(unsigned int)index != index) { + index = (unsigned int)index_long; + + if ((unsigned long)index != index_long) { PyErr_SetString(PyExc_OverflowError, "index is too large"); return NULL; } From 6ef546a89c279699d989b12dab2ebbf828118976 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 25 Jun 2022 12:35:45 +0300 Subject: [PATCH 4/9] Make test meaningful. --- Lib/test/test_socket.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 79794ef40e8e07..6b1f37d9e4e6d9 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1081,12 +1081,16 @@ def testInterfaceNameIndex(self): 'socket.if_indextoname() not available.') def testInvalidInterfaceIndexToName(self): self.assertRaises(OSError, socket.if_indextoname, 0) + self.assertRaises(OverflowError, socket.if_indextoname, -1) + self.assertRaises(OverflowError, socket.if_indextoname, 2**1000) self.assertRaises(TypeError, socket.if_indextoname, '_DEADBEEF') - for i in 2**32-1, 2**32, 2**64-1, 2**64: - try: - socket.if_indextoname(i) - except (OSError, OverflowError): - pass + if hasattr(socket, 'if_nameindex'): + indices = dict(socket.if_nameindex()) + for index in indices: + index2 = index + 2**32 + if index2 not in indices: + with self.assertRaises((OverflowError, OSError)): + socket.if_indextoname(index2) @unittest.skipUnless(hasattr(socket, 'if_nametoindex'), 'socket.if_nametoindex() not available.') From 636014d86839d92382e07291ebcd7200f76fe7b6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 25 Jun 2022 12:37:54 +0300 Subject: [PATCH 5/9] Make the code more future proof on Windows. --- Modules/socketmodule.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 1d95024c7778d2..ac8327c2bfc988 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -6955,13 +6955,7 @@ Returns the interface index corresponding to the interface name if_name."); static PyObject * socket_if_indextoname(PyObject *self, PyObject *arg) { -#ifdef MS_WINDOWS - NET_IFINDEX index_long; - NET_IFINDEX index; -#else unsigned long index_long; - unsigned int index; -#endif char name[IF_NAMESIZE + 1]; index_long = PyLong_AsUnsignedLong(arg); @@ -6969,7 +6963,11 @@ socket_if_indextoname(PyObject *self, PyObject *arg) return NULL; } - index = (unsigned int)index_long; +#ifdef MS_WINDOWS + NET_IFINDEX index = (NET_IFINDEX)index_long; +#else + unsigned int index = (unsigned int)index_long; +#endif if ((unsigned long)index != index_long) { PyErr_SetString(PyExc_OverflowError, "index is too large"); From c865fe773d385aca42be23f8da3d54227dcdf5e9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 1 Dec 2023 16:10:25 +0200 Subject: [PATCH 6/9] Add a NEWS entry. --- .../next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst diff --git a/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst b/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst new file mode 100644 index 00000000000000..37a79db469f2ce --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst @@ -0,0 +1,2 @@ +Fix an integer overflow in :func:`socket.if_indextoname` on 64-bit +non-Windows platforms. From 4a60c85c53cb400dcdbcbada3667ddcf9bc0e0d2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 1 Dec 2023 16:28:10 +0200 Subject: [PATCH 7/9] Add crashing tests. --- Lib/test/test_socket.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 872dd2c72cfe24..4eb5af99d6674c 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1092,6 +1092,10 @@ def testInvalidInterfaceIndexToName(self): if index2 not in indices: with self.assertRaises((OverflowError, OSError)): socket.if_indextoname(index2) + for index in 2**32-1, 2**64-1: + if index not in indices: + with self.assertRaises((OverflowError, OSError)): + socket.if_indextoname(index) @unittest.skipUnless(hasattr(socket, 'if_nametoindex'), 'socket.if_nametoindex() not available.') From 94ea23c550dfde501dbabb15bc88d7dca7ef65e6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 1 Dec 2023 16:32:06 +0200 Subject: [PATCH 8/9] Update the NEWS entry. --- .../next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst b/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst index 37a79db469f2ce..feb7a8643b97f6 100644 --- a/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst +++ b/Misc/NEWS.d/next/Library/2023-12-01-16-09-59.gh-issue-81194.FFad1c.rst @@ -1,2 +1,3 @@ +Fix a crash in :func:`socket.if_indextoname` with specific value (UINT_MAX). Fix an integer overflow in :func:`socket.if_indextoname` on 64-bit non-Windows platforms. From 5addb69f04e24b83ed1a644be0b36274e79fd59b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 1 Dec 2023 16:40:01 +0200 Subject: [PATCH 9/9] Address style suggestion. --- Modules/socketmodule.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 7ec9fe553e6179..0a0e0e78656f76 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -7071,10 +7071,7 @@ _socket_socket_if_nametoindex_impl(PySocketSockObject *self, PyObject *oname) static PyObject * socket_if_indextoname(PyObject *self, PyObject *arg) { - unsigned long index_long; - char name[IF_NAMESIZE + 1]; - - index_long = PyLong_AsUnsignedLong(arg); + unsigned long index_long = PyLong_AsUnsignedLong(arg); if (index_long == (unsigned long) -1 && PyErr_Occurred()) { return NULL; } @@ -7090,6 +7087,7 @@ socket_if_indextoname(PyObject *self, PyObject *arg) return NULL; } + char name[IF_NAMESIZE + 1]; if (if_indextoname(index, name) == NULL) { PyErr_SetFromErrno(PyExc_OSError); return NULL;