[issue8372] socket: Buffer overrun while reading unterminated AF_UNIX addresses
David Watson
report at bugs.python.org
Mon Sep 6 18:42:16 CEST 2010
David Watson <baikie at users.sourceforge.net> added the comment:
> baikie, coming back to your original message: what precisely makes you believe that sun_path does not need to be null-terminated on Linux?
That's the way I demonstrated the bug - the only way to bind to a
108-byte path is to pass it without null termination, because
Linux will not accept an oversized sockaddr_un structure (e.g. a
108-byte path plus null terminator). Also, unless it's on OS/2,
Python's existing code never includes the null terminator in the
address size it passes to the system call, so a correctly-
behaving OS should never see it.
However, it does now occur to me that a replacement libc
implementation for Linux could try to do something with sun_path
during the call and assume it's null-terminated even though the
kernel doesn't, so it may be best to keep the null termination
requirement after all. In that case, there would be no way to
test for the bug from within Python, so the test problems would
be somewhat moot (although the test code could still be used by
changing UNIX_PATH_MAX from 108 to 107).
Attaching four-space-indent versions of the original patches (for
2.x and 3.x), and tests incorporating Antoine's use of
assertRaisesRegexp.
----------
Added file: http://bugs.python.org/file18770/linux-pass-unterminated-4spc.diff
Added file: http://bugs.python.org/file18771/return-unterminated-2.x-4spc.diff
Added file: http://bugs.python.org/file18772/return-unterminated-3.x-4spc.diff
Added file: http://bugs.python.org/file18773/addrlen-2.x-4spc.diff
Added file: http://bugs.python.org/file18774/addrlen-3.x-4spc.diff
Added file: http://bugs.python.org/file18775/test-2.x-new.diff
Added file: http://bugs.python.org/file18776/test-3.x-new.diff
_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue8372>
_______________________________________
-------------- next part --------------
Allow AF_UNIX pathnames up to the maximum 108 bytes on Linux,
since it does not require sun_path to be null terminated.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -1187,27 +1187,16 @@ getsockaddrarg(PySocketSockObject *s, Py
addr = (struct sockaddr_un*)addr_ret;
#ifdef linux
- if (len > 0 && path[0] == 0) {
- /* Linux abstract namespace extension */
- if (len > sizeof addr->sun_path) {
- PyErr_SetString(socket_error,
- "AF_UNIX path too long");
- return 0;
- }
- }
- else
-#endif /* linux */
- {
- /* regular NULL-terminated string */
- if (len >= sizeof addr->sun_path) {
- PyErr_SetString(socket_error,
- "AF_UNIX path too long");
- return 0;
- }
- addr->sun_path[len] = 0;
+ if (len > sizeof(addr->sun_path)) {
+#else
+ if (len >= sizeof(addr->sun_path)) {
+#endif
+ PyErr_SetString(socket_error, "AF_UNIX path too long");
+ return 0;
}
addr->sun_family = s->sock_family;
memcpy(addr->sun_path, path, len);
+ memset(addr->sun_path + len, 0, sizeof(addr->sun_path) - len);
#if defined(PYOS_OS2)
*len_ret = sizeof(*addr);
#else
-------------- next part --------------
When parsing sockaddr_un structures returned by accept(), etc.,
only examine bytes up to supplied addrlen and do not require null
termination.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -998,19 +998,22 @@ makesockaddr(int sockfd, struct sockaddr
#if defined(AF_UNIX)
case AF_UNIX:
{
+ Py_ssize_t len, splen;
struct sockaddr_un *a = (struct sockaddr_un *) addr;
+ splen = addrlen - offsetof(struct sockaddr_un, sun_path);
#ifdef linux
- if (a->sun_path[0] == 0) { /* Linux abstract namespace */
- addrlen -= offsetof(struct sockaddr_un, sun_path);
- return PyString_FromStringAndSize(a->sun_path,
- addrlen);
+ if (splen > 0 && a->sun_path[0] == 0) {
+ /* Linux abstract namespace */
+ len = splen;
}
else
#endif /* linux */
{
- /* regular NULL-terminated string */
- return PyString_FromString(a->sun_path);
+ /* String, up to null terminator if present */
+ for (len = 0; len < splen && a->sun_path[len] != 0; len++)
+ ;
}
+ return PyString_FromStringAndSize(a->sun_path, len);
}
#endif /* AF_UNIX */
-------------- next part --------------
When parsing sockaddr_un structures returned by accept(), etc.,
only examine bytes up to supplied addrlen and do not require null
termination.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -962,18 +962,22 @@ makesockaddr(SOCKET_T sockfd, struct soc
#if defined(AF_UNIX)
case AF_UNIX:
{
+ Py_ssize_t len, splen;
struct sockaddr_un *a = (struct sockaddr_un *) addr;
+ splen = addrlen - offsetof(struct sockaddr_un, sun_path);
#ifdef linux
- if (a->sun_path[0] == 0) { /* Linux abstract namespace */
- addrlen -= offsetof(struct sockaddr_un, sun_path);
- return PyBytes_FromStringAndSize(a->sun_path, addrlen);
+ if (splen > 0 && a->sun_path[0] == 0) {
+ /* Linux abstract namespace */
+ return PyBytes_FromStringAndSize(a->sun_path, splen);
}
else
#endif /* linux */
{
- /* regular NULL-terminated string */
- return PyUnicode_FromString(a->sun_path);
+ /* String, up to null terminator if present */
+ for (len = 0; len < splen && a->sun_path[len] != 0; len++)
+ ;
}
+ return PyUnicode_FromStringAndSize(a->sun_path, len);
}
#endif /* AF_UNIX */
-------------- next part --------------
If accept(), etc. return a larger addrlen than was supplied,
ignore it and use the original buffer length.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -1608,6 +1608,7 @@ sock_accept(PySocketSockObject *s)
sock_addr_t addrbuf;
SOCKET_T newfd;
socklen_t addrlen;
+ socklen_t buflen;
PyObject *sock = NULL;
PyObject *addr = NULL;
PyObject *res = NULL;
@@ -1615,6 +1616,7 @@ sock_accept(PySocketSockObject *s)
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
#ifdef MS_WINDOWS
@@ -1656,7 +1658,7 @@ sock_accept(PySocketSockObject *s)
goto finally;
}
addr = makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
- addrlen, s->sock_proto);
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
if (addr == NULL)
goto finally;
@@ -2154,17 +2156,19 @@ sock_getsockname(PySocketSockObject *s)
sock_addr_t addrbuf;
int res;
socklen_t addrlen;
+ socklen_t buflen;
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
Py_BEGIN_ALLOW_THREADS
res = getsockname(s->sock_fd, SAS2SA(&addrbuf), &addrlen);
Py_END_ALLOW_THREADS
if (res < 0)
return s->errorhandler();
- return makesockaddr(s->sock_fd, SAS2SA(&addrbuf), addrlen,
- s->sock_proto);
+ return makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
}
PyDoc_STRVAR(getsockname_doc,
@@ -2183,17 +2187,19 @@ sock_getpeername(PySocketSockObject *s)
sock_addr_t addrbuf;
int res;
socklen_t addrlen;
+ socklen_t buflen;
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
Py_BEGIN_ALLOW_THREADS
res = getpeername(s->sock_fd, SAS2SA(&addrbuf), &addrlen);
Py_END_ALLOW_THREADS
if (res < 0)
return s->errorhandler();
- return makesockaddr(s->sock_fd, SAS2SA(&addrbuf), addrlen,
- s->sock_proto);
+ return makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
}
PyDoc_STRVAR(getpeername_doc,
@@ -2515,11 +2521,13 @@ sock_recvfrom_guts(PySocketSockObject *s
int timeout;
ssize_t n = -1;
socklen_t addrlen;
+ socklen_t buflen;
*addr = NULL;
if (!getsockaddrlen(s, &addrlen))
return -1;
+ buflen = addrlen;
if (!IS_SELECTABLE(s)) {
select_error();
@@ -2555,7 +2563,8 @@ sock_recvfrom_guts(PySocketSockObject *s
}
if (!(*addr = makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
- addrlen, s->sock_proto)))
+ (addrlen > buflen) ? buflen : addrlen,
+ s->sock_proto)))
return -1;
return n;
-------------- next part --------------
If accept(), etc. return a larger addrlen than was supplied,
ignore it and use the original buffer length.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -1572,6 +1572,7 @@ sock_accept(PySocketSockObject *s)
sock_addr_t addrbuf;
SOCKET_T newfd = INVALID_SOCKET;
socklen_t addrlen;
+ socklen_t buflen;
PyObject *sock = NULL;
PyObject *addr = NULL;
PyObject *res = NULL;
@@ -1579,6 +1580,7 @@ sock_accept(PySocketSockObject *s)
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
if (!IS_SELECTABLE(s))
@@ -1605,7 +1607,7 @@ sock_accept(PySocketSockObject *s)
}
addr = makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
- addrlen, s->sock_proto);
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
if (addr == NULL)
goto finally;
@@ -2055,17 +2057,19 @@ sock_getsockname(PySocketSockObject *s)
sock_addr_t addrbuf;
int res;
socklen_t addrlen;
+ socklen_t buflen;
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
Py_BEGIN_ALLOW_THREADS
res = getsockname(s->sock_fd, SAS2SA(&addrbuf), &addrlen);
Py_END_ALLOW_THREADS
if (res < 0)
return s->errorhandler();
- return makesockaddr(s->sock_fd, SAS2SA(&addrbuf), addrlen,
- s->sock_proto);
+ return makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
}
PyDoc_STRVAR(getsockname_doc,
@@ -2084,17 +2088,19 @@ sock_getpeername(PySocketSockObject *s)
sock_addr_t addrbuf;
int res;
socklen_t addrlen;
+ socklen_t buflen;
if (!getsockaddrlen(s, &addrlen))
return NULL;
+ buflen = addrlen;
memset(&addrbuf, 0, addrlen);
Py_BEGIN_ALLOW_THREADS
res = getpeername(s->sock_fd, SAS2SA(&addrbuf), &addrlen);
Py_END_ALLOW_THREADS
if (res < 0)
return s->errorhandler();
- return makesockaddr(s->sock_fd, SAS2SA(&addrbuf), addrlen,
- s->sock_proto);
+ return makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
+ (addrlen > buflen) ? buflen : addrlen, s->sock_proto);
}
PyDoc_STRVAR(getpeername_doc,
@@ -2354,11 +2360,13 @@ sock_recvfrom_guts(PySocketSockObject *s
int timeout;
Py_ssize_t n = -1;
socklen_t addrlen;
+ socklen_t buflen;
*addr = NULL;
if (!getsockaddrlen(s, &addrlen))
return -1;
+ buflen = addrlen;
if (!IS_SELECTABLE(s)) {
select_error();
@@ -2394,7 +2402,8 @@ sock_recvfrom_guts(PySocketSockObject *s
}
if (!(*addr = makesockaddr(s->sock_fd, SAS2SA(&addrbuf),
- addrlen, s->sock_proto)))
+ (addrlen > buflen) ? buflen : addrlen,
+ s->sock_proto)))
return -1;
return n;
-------------- next part --------------
Test handling of unterminated AF_UNIX addresses on Linux.
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -1261,6 +1261,43 @@ class TestExceptions(unittest.TestCase):
self.assertTrue(issubclass(socket.gaierror, socket.error))
self.assertTrue(issubclass(socket.timeout, socket.error))
+class TestLinuxPathLen(unittest.TestCase):
+
+ # Test AF_UNIX path length limits on Linux.
+
+ UNIX_PATH_MAX = 108
+
+ def setUp(self):
+ self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ self.to_unlink = []
+
+ def tearDown(self):
+ self.sock.close()
+ for name in self.to_unlink:
+ test_support.unlink(name)
+
+ def pathname(self, length):
+ # Return a pathname of the given length.
+ path = os.path.abspath(test_support.TESTFN)
+ return path + "a" * (length - len(path))
+
+ def testPathTooLong(self):
+ # Check we can't bind to a path longer than the assumed maximum.
+ path = self.pathname(self.UNIX_PATH_MAX + 1)
+ with self.assertRaisesRegexp(socket.error, "AF_UNIX path too long"):
+ self.sock.bind(path)
+ self.to_unlink.append(path)
+
+ def testMaxPathLen(self):
+ # Test binding to a path of the maximum length and reading the
+ # address back. In this case, sun_path is not null terminated,
+ # and makesockaddr() used to read past the end of it.
+ path = self.pathname(self.UNIX_PATH_MAX)
+ self.sock.bind(path)
+ self.to_unlink.append(path)
+ self.assertEqual(self.sock.getsockname(), path)
+ os.stat(path)
+
class TestLinuxAbstractNamespace(unittest.TestCase):
UNIX_PATH_MAX = 108
@@ -1456,6 +1493,7 @@ def test_main():
tests.append(BasicSocketPairTest)
if sys.platform == 'linux2':
tests.append(TestLinuxAbstractNamespace)
+ tests.append(TestLinuxPathLen)
if isTipcAvailable():
tests.append(TIPCTest)
tests.append(TIPCThreadableTest)
-------------- next part --------------
Test handling of unterminated AF_UNIX addresses on Linux.
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -1392,6 +1392,53 @@ class TestExceptions(unittest.TestCase):
self.assertTrue(issubclass(socket.gaierror, socket.error))
self.assertTrue(issubclass(socket.timeout, socket.error))
+class TestLinuxPathLen(unittest.TestCase):
+
+ # Test AF_UNIX path length limits on Linux.
+
+ UNIX_PATH_MAX = 108
+
+ def setUp(self):
+ self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ self.to_unlink = []
+
+ def tearDown(self):
+ self.sock.close()
+ for name in self.to_unlink:
+ support.unlink(name)
+
+ def pathEncodingArgs(self):
+ # Return the encoding and error handler used to encode/decode
+ # pathnames.
+ encoding = sys.getfilesystemencoding()
+ if encoding is None:
+ encoding = sys.getdefaultencoding()
+ return encoding, "surrogateescape"
+
+ def pathname(self, length):
+ # Return a bytes pathname of the given length.
+ path = os.path.abspath(support.TESTFN)
+ path_bytes = path.encode(*self.pathEncodingArgs())
+ return path_bytes + b"a" * (length - len(path_bytes))
+
+ def testPathTooLong(self):
+ # Check we can't bind to a path longer than the assumed maximum.
+ path = self.pathname(self.UNIX_PATH_MAX + 1)
+ with self.assertRaisesRegexp(socket.error, "AF_UNIX path too long"):
+ self.sock.bind(path)
+ self.to_unlink.append(path)
+
+ def testMaxPathLen(self):
+ # Test binding to a path of the maximum length and reading the
+ # address back. In this case, sun_path is not null terminated,
+ # and makesockaddr() used to read past the end of it.
+ path = self.pathname(self.UNIX_PATH_MAX)
+ self.sock.bind(path)
+ self.to_unlink.append(path)
+ self.assertEqual(self.sock.getsockname(),
+ path.decode(*self.pathEncodingArgs()))
+ os.stat(path)
+
class TestLinuxAbstractNamespace(unittest.TestCase):
UNIX_PATH_MAX = 108
@@ -1583,6 +1630,7 @@ def test_main():
tests.append(BasicSocketPairTest)
if sys.platform == 'linux2':
tests.append(TestLinuxAbstractNamespace)
+ tests.append(TestLinuxPathLen)
if isTipcAvailable():
tests.append(TIPCTest)
tests.append(TIPCThreadableTest)
More information about the Python-bugs-list
mailing list