From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7097E469719 for ; Fri, 21 Feb 2020 15:32:05 +0300 (MSK) From: Chris Sosnin Date: Fri, 21 Feb 2020 15:32:03 +0300 Message-Id: <20200221123203.6956-1-k.sosnin@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] app: verify unix socket path length in socket.tcp_server() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Providing socket pathname longer than UNIX_PATH_MAX to socket.tcp_server() will not cause any error, lbox_socket_local_resolve will just truncate the name according to the limit, causing bad behavior (tarantool will try to access a socket, which doesn't exist). Thus, let's verify, that pathname can fit into buffer. Closes #4634 --- branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4634-verify-socket-path-length issue: https://github.com/tarantool/tarantool/issues/4634 The issue itself already contains the fix, I just added the test and dropped the following comment from the code: + /* + * sun_path doesn't have to be null-terminated on + * Linux, but we ensure it is so for maximum + * portability. + */ According to unix(7): Pathname sockets When binding a socket to a pathname, a few rules should be observed for maximum portability and ease of coding: * The pathname in sun_path should be null-terminated. So such comment (at least for me) would rather be confusing. src/lua/socket.c | 3 ++- test/app/socket.result | 8 ++++++++ test/app/socket.test.lua | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/lua/socket.c b/src/lua/socket.c index 130378caf..e75a8802e 100644 --- a/src/lua/socket.c +++ b/src/lua/socket.c @@ -403,7 +403,8 @@ lbox_socket_local_resolve(const char *host, const char *port, { if (strcmp(host, "unix/") == 0) { struct sockaddr_un *uaddr = (struct sockaddr_un *) addr; - if (*socklen < sizeof(*uaddr)) { + if (*socklen < sizeof(*uaddr) || + strlen(port) >= sizeof(uaddr->sun_path)) { errno = ENOBUFS; return -1; } diff --git a/test/app/socket.result b/test/app/socket.result index 9829df138..f47d06935 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -1614,6 +1614,14 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er --- - true ... +-- gh-4634: verify socket path length in socket.tcp_server. +long_port = string.rep('a', 110) +--- +... +socket.tcp_server('unix/', long_port, function(s) end) == errno.ENOBUF +--- +- true +... -- gh-574: check that fiber with getaddrinfo can be safely cancelled test_run:cmd("setopt delimiter ';'") --- diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index af926c35b..e303d3743 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -531,6 +531,10 @@ socket.getaddrinfo('host', 'port', { family = 'WRONG' }) == nil and errno() == e socket.getaddrinfo('host', 'port', { protocol = 'WRONG' }) == nil and errno() == errno.EPROTOTYPE socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == errno.EINVAL +-- gh-4634: verify socket path length in socket.tcp_server. +long_port = string.rep('a', 110) +socket.tcp_server('unix/', long_port, function(s) end) == errno.ENOBUF + -- gh-574: check that fiber with getaddrinfo can be safely cancelled test_run:cmd("setopt delimiter ';'") f = fiber.create(function() -- 2.21.1 (Apple Git-122.3)