From 88fc5c2b23baae9b7a1b564732ebb7e2c9c854cf Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 24 Mar 2025 14:47:13 +0100 Subject: [PATCH 01/12] gh-111178: Fix getsockaddrarg() undefined behavior Don't pass direct references to sockaddr members since their type may not match PyArg_ParseTuple() types. Instead, use temporary 'int' and 'unsigned char' variables, and update sockaddr members afterwards. --- Modules/socketmodule.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 795451d48e7b35..662ae7b1f22c61 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2045,14 +2045,21 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, memset(addr, 0, sizeof(struct sockaddr_l2)); _BT_L2_MEMB(addr, family) = AF_BLUETOOTH; _BT_L2_MEMB(addr, bdaddr_type) = BDADDR_BREDR; + int psm; + int cid = _BT_L2_MEMB(addr, cid); + unsigned char bdaddr_type = _BT_L2_MEMB(addr, bdaddr_type); if (!PyArg_ParseTuple(args, "si|iB", &straddr, - &_BT_L2_MEMB(addr, psm), - &_BT_L2_MEMB(addr, cid), - &_BT_L2_MEMB(addr, bdaddr_type))) { + &psm, + &cid, + &bdaddr_type)) { PyErr_Format(PyExc_OSError, "%s(): wrong format", caller); return 0; } + _BT_L2_MEMB(addr, psm) = psm; + _BT_L2_MEMB(addr, cid) = cid; + _BT_L2_MEMB(addr, bdaddr_type) = bdaddr_type; + if (setbdaddr(straddr, &_BT_L2_MEMB(addr, bdaddr)) < 0) return 0; From 1aebf4c908b9fbac3d5c921c7977d9d1c36dc229 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 24 Mar 2025 15:28:48 +0100 Subject: [PATCH 02/12] Simplify code --- Modules/socketmodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 662ae7b1f22c61..38ef7a8bb40b72 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2044,10 +2044,9 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, struct sockaddr_l2 *addr = &addrbuf->bt_l2; memset(addr, 0, sizeof(struct sockaddr_l2)); _BT_L2_MEMB(addr, family) = AF_BLUETOOTH; - _BT_L2_MEMB(addr, bdaddr_type) = BDADDR_BREDR; int psm; int cid = _BT_L2_MEMB(addr, cid); - unsigned char bdaddr_type = _BT_L2_MEMB(addr, bdaddr_type); + unsigned char bdaddr_type = BDADDR_BREDR; if (!PyArg_ParseTuple(args, "si|iB", &straddr, &psm, &cid, From 9eb932c470e45e4652e2ebefbcea63227e18b33b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 24 Mar 2025 15:30:52 +0100 Subject: [PATCH 03/12] Initialize cid to 0 --- Modules/socketmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 38ef7a8bb40b72..27fc9909072e83 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2045,7 +2045,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, memset(addr, 0, sizeof(struct sockaddr_l2)); _BT_L2_MEMB(addr, family) = AF_BLUETOOTH; int psm; - int cid = _BT_L2_MEMB(addr, cid); + int cid = 0; unsigned char bdaddr_type = BDADDR_BREDR; if (!PyArg_ParseTuple(args, "si|iB", &straddr, &psm, From 533a4784c6e82cbd0720deb3c91726d15b42fa21 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 28 Mar 2025 11:17:42 +0100 Subject: [PATCH 04/12] Use 'unsigned short' type Fix also BTPROTO_RFCOMM and BTPROTO_HCI code path. --- Modules/socketmodule.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 27fc9909072e83..aacc965299dce8 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2044,10 +2044,10 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, struct sockaddr_l2 *addr = &addrbuf->bt_l2; memset(addr, 0, sizeof(struct sockaddr_l2)); _BT_L2_MEMB(addr, family) = AF_BLUETOOTH; - int psm; - int cid = 0; + unsigned short psm; + unsigned short cid = 0; unsigned char bdaddr_type = BDADDR_BREDR; - if (!PyArg_ParseTuple(args, "si|iB", &straddr, + if (!PyArg_ParseTuple(args, "sH|HB", &straddr, &psm, &cid, &bdaddr_type)) { @@ -2071,12 +2071,15 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, const char *straddr; struct sockaddr_rc *addr = &addrbuf->bt_rc; _BT_RC_MEMB(addr, family) = AF_BLUETOOTH; - if (!PyArg_ParseTuple(args, "si", &straddr, - &_BT_RC_MEMB(addr, channel))) { + uint8_t channel = _BT_RC_MEMB(addr, channel); + if (!PyArg_ParseTuple(args, "sB", &straddr, + &channel)) { PyErr_Format(PyExc_OSError, "%s(): wrong format", caller); return 0; } + _BT_RC_MEMB(addr, channel) = channel; + if (setbdaddr(straddr, &_BT_RC_MEMB(addr, bdaddr)) < 0) return 0; @@ -2100,11 +2103,13 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, return 0; #else /* __NetBSD__ || __DragonFly__ */ _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; - if (!PyArg_ParseTuple(args, "i", &_BT_HCI_MEMB(addr, dev))) { + unsigned short dev = _BT_HCI_MEMB(addr, dev); + if (!PyArg_ParseTuple(args, "H", &dev)) { PyErr_Format(PyExc_OSError, "%s(): wrong format", caller); return 0; } + _BT_HCI_MEMB(addr, dev) = dev; #endif /* !(__NetBSD__ || __DragonFly__) */ *len_ret = sizeof *addr; return 1; From 9968c1eb4bea35cce8f3044c6a468ef8db51b4f4 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 28 Mar 2025 11:23:19 +0100 Subject: [PATCH 05/12] Use 'unsigned char' --- Modules/socketmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index aacc965299dce8..20e6e6501a8419 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2071,7 +2071,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, const char *straddr; struct sockaddr_rc *addr = &addrbuf->bt_rc; _BT_RC_MEMB(addr, family) = AF_BLUETOOTH; - uint8_t channel = _BT_RC_MEMB(addr, channel); + unsigned char channel = _BT_RC_MEMB(addr, channel); if (!PyArg_ParseTuple(args, "sB", &straddr, &channel)) { PyErr_Format(PyExc_OSError, From a95b22d5651e055ae29df728a27170016ef18b9f Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 28 Mar 2025 11:26:33 +0100 Subject: [PATCH 06/12] Add NEWS entry --- .../next/Library/2025-03-28-11-26-31.gh-issue-131668.tcS4xS.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-03-28-11-26-31.gh-issue-131668.tcS4xS.rst diff --git a/Misc/NEWS.d/next/Library/2025-03-28-11-26-31.gh-issue-131668.tcS4xS.rst b/Misc/NEWS.d/next/Library/2025-03-28-11-26-31.gh-issue-131668.tcS4xS.rst new file mode 100644 index 00000000000000..ec04cdd30a2f28 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-03-28-11-26-31.gh-issue-131668.tcS4xS.rst @@ -0,0 +1 @@ +:mod:`socket`: Fix code parsing AF_BLUETOOTH socket addresses. From 18ed09edd9b69938ce3d92d1edc28d05bdf30148 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 28 Mar 2025 16:18:42 +0100 Subject: [PATCH 07/12] Use 'unsigned long' to parse channel on Windows --- Modules/socketmodule.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 20e6e6501a8419..e54f4e9ba6bd23 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2071,13 +2071,19 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, const char *straddr; struct sockaddr_rc *addr = &addrbuf->bt_rc; _BT_RC_MEMB(addr, family) = AF_BLUETOOTH; +#ifdef MS_WINDOWS + unsigned long channel = _BT_RC_MEMB(addr, channel); +# define FORMAT_CHANNEL "k" +#else unsigned char channel = _BT_RC_MEMB(addr, channel); - if (!PyArg_ParseTuple(args, "sB", &straddr, - &channel)) { - PyErr_Format(PyExc_OSError, - "%s(): wrong format", caller); +# define FORMAT_CHANNEL "B" +#endif + if (!PyArg_ParseTuple(args, "s" FORMAT_CHANNEL, + &straddr, &channel)) { + PyErr_Format(PyExc_OSError, "%s(): wrong format", caller); return 0; } +#undef FORMAT_CHANNEL _BT_RC_MEMB(addr, channel) = channel; if (setbdaddr(straddr, &_BT_RC_MEMB(addr, bdaddr)) < 0) From 99dcf45d39e64e2065f983f6a3972b554981cf06 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 1 Apr 2025 11:10:49 +0200 Subject: [PATCH 08/12] Fix BTPROTO_HCI on FreeBSD --- Modules/socketmodule.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index e54f4e9ba6bd23..ef523c3cffba76 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2107,6 +2107,21 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, straddr = PyBytes_AS_STRING(args); if (setbdaddr(straddr, &_BT_HCI_MEMB(addr, bdaddr)) < 0) return 0; +#elif defined(__FreeBSD__) + _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; + if (!PyBytes_Check(args)) { + PyErr_Format(PyExc_OSError, "%s: " + "wrong node format", caller); + return 0; + } + const char *straddr = PyBytes_AS_STRING(args); + size_t len = PyBytes_GET_SIZE(args); + if (len >= sizeof(_BT_HCI_MEMB(addr, node))) { + PyErr_Format(PyExc_OSError, "%s: " + "node too long", caller); + return 0; + } + strcpy(_BT_HCI_MEMB(addr, node), straddr); #else /* __NetBSD__ || __DragonFly__ */ _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; unsigned short dev = _BT_HCI_MEMB(addr, dev); From c0d7d039ccc6379b549d5c200055e104c4d5a21e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 1 Apr 2025 12:31:48 +0200 Subject: [PATCH 09/12] Update BTPROTO_HCI code --- Modules/socketmodule.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index ef523c3cffba76..768ead6a28f30b 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -1521,11 +1521,15 @@ makesockaddr(SOCKET_T sockfd, struct sockaddr *addr, size_t addrlen, int proto) struct sockaddr_hci *a = (struct sockaddr_hci *) addr; #if defined(__NetBSD__) || defined(__DragonFly__) return makebdaddr(&_BT_HCI_MEMB(a, bdaddr)); -#else /* __NetBSD__ || __DragonFly__ */ +#elif defined(__FreeBSD__) + char *node = _BT_HCI_MEMB(a, node); + size_t len = strnlen(node, sizeof(_BT_HCI_MEMB(a, node))); + return PyBytes_FromStringAndSize(node, (Py_ssize_t)len); +#else PyObject *ret = NULL; ret = Py_BuildValue("i", _BT_HCI_MEMB(a, dev)); return ret; -#endif /* !(__NetBSD__ || __DragonFly__) */ +#endif } #if !defined(__FreeBSD__) @@ -2116,12 +2120,18 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, } const char *straddr = PyBytes_AS_STRING(args); size_t len = PyBytes_GET_SIZE(args); - if (len >= sizeof(_BT_HCI_MEMB(addr, node))) { + if (strlen(straddr) != len) { + PyErr_Format(PyExc_OSError, "%s: " + "node contains embedded null character", caller); + return 0; + } + if (len > sizeof(_BT_HCI_MEMB(addr, node))) { PyErr_Format(PyExc_OSError, "%s: " "node too long", caller); return 0; } - strcpy(_BT_HCI_MEMB(addr, node), straddr); + strncpy(_BT_HCI_MEMB(addr, node), straddr, + sizeof(_BT_HCI_MEMB(addr, node))); #else /* __NetBSD__ || __DragonFly__ */ _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; unsigned short dev = _BT_HCI_MEMB(addr, dev); From 7178cfd4bfd604d0322123d3996c5c52ce7d7831 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 1 Apr 2025 15:13:13 +0200 Subject: [PATCH 10/12] Change exception type to ValueError --- Modules/socketmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 768ead6a28f30b..6fb04ba38e5da7 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2121,12 +2121,12 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, const char *straddr = PyBytes_AS_STRING(args); size_t len = PyBytes_GET_SIZE(args); if (strlen(straddr) != len) { - PyErr_Format(PyExc_OSError, "%s: " + PyErr_Format(PyExc_ValueError, "%s: " "node contains embedded null character", caller); return 0; } if (len > sizeof(_BT_HCI_MEMB(addr, node))) { - PyErr_Format(PyExc_OSError, "%s: " + PyErr_Format(PyExc_ValueError, "%s: " "node too long", caller); return 0; } From 94f7b6f826b473999b59e529175a0c26ad46e9bf Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 1 Apr 2025 15:27:40 +0200 Subject: [PATCH 11/12] Remove outdated comments --- Modules/socketmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 6fb04ba38e5da7..3f006792a1ed7a 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2132,7 +2132,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, } strncpy(_BT_HCI_MEMB(addr, node), straddr, sizeof(_BT_HCI_MEMB(addr, node))); -#else /* __NetBSD__ || __DragonFly__ */ +#else _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; unsigned short dev = _BT_HCI_MEMB(addr, dev); if (!PyArg_ParseTuple(args, "H", &dev)) { @@ -2141,7 +2141,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, return 0; } _BT_HCI_MEMB(addr, dev) = dev; -#endif /* !(__NetBSD__ || __DragonFly__) */ +#endif *len_ret = sizeof *addr; return 1; } From 7455bf5aed2607cab7c34c8993bd649a3ec6a17c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 1 Apr 2025 15:28:41 +0200 Subject: [PATCH 12/12] Remove useless assignment The socket address channel is not initialized before calling PyArg_ParseTuple(). --- Modules/socketmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 3f006792a1ed7a..6f0d2579cb4025 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2076,10 +2076,10 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, struct sockaddr_rc *addr = &addrbuf->bt_rc; _BT_RC_MEMB(addr, family) = AF_BLUETOOTH; #ifdef MS_WINDOWS - unsigned long channel = _BT_RC_MEMB(addr, channel); + unsigned long channel; # define FORMAT_CHANNEL "k" #else - unsigned char channel = _BT_RC_MEMB(addr, channel); + unsigned char channel; # define FORMAT_CHANNEL "B" #endif if (!PyArg_ParseTuple(args, "s" FORMAT_CHANNEL,