From 66319cf494d7875a6ffc8653928d31ffbd07187f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 28 Apr 2017 01:25:00 +0200 Subject: [PATCH] unix: fail with ENAMETOOLONG in `uv_pipe_*` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fail with ENAMETOOLONG when the name of a Unix socket exceeds `sizeof(saddr.sun_path)`. Previously the path was just truncated, which could result in nasty bugs, and even though that behaviour has been always been around, it’s hard to imagine a situation in which ending up with an incorrect path is better than not creating a socket at all. Refs: https://github.com/nodejs/node/pull/12601 Refs: https://github.com/nodejs/node/issues/12708 PR-URL: https://github.com/libuv/libuv/pull/1329 Reviewed-By: Santiago Gimeno Reviewed-By: Colin Ihrig Reviewed-By: Saúl Ibarra Corretgé --- docs/src/pipe.rst | 12 ++++++++---- src/unix/pipe.c | 19 +++++++++++++++---- test/test-list.h | 12 ++++++++++++ test/test-pipe-bind-error.c | 25 +++++++++++++++++++++++++ test/test-pipe-connect-error.c | 31 +++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 8 deletions(-) diff --git a/docs/src/pipe.rst b/docs/src/pipe.rst index d33b0f2b..9c823491 100644 --- a/docs/src/pipe.rst +++ b/docs/src/pipe.rst @@ -49,16 +49,20 @@ API Bind the pipe to a file path (Unix) or a name (Windows). .. note:: - Paths on Unix get truncated to ``sizeof(sockaddr_un.sun_path)`` bytes, typically between - 92 and 108 bytes. + If a path on Unix exceeds ``sizeof(sockaddr_un.sun_path)`` bytes, typically between + 92 and 108 bytes, ``uv_pipe_bind`` will fail with ``UV_ENAMETOOLONG``. + + .. versionchanged: 2.0.0 long filenames will lead to an error rather than being truncated .. c:function:: void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, const char* name, uv_connect_cb cb) Connect to the Unix domain socket or the named pipe. .. note:: - Paths on Unix get truncated to ``sizeof(sockaddr_un.sun_path)`` bytes, typically between - 92 and 108 bytes. + If a path on Unix exceeds ``sizeof(sockaddr_un.sun_path)`` bytes, typically between + 92 and 108 bytes, ``uv_pipe_bind`` will fail with ``UV_ENAMETOOLONG``. + + .. versionchanged: 2.0.0 long filenames will lead to an error rather than being truncated .. c:function:: int uv_pipe_getsockname(const uv_pipe_t* handle, char* buffer, size_t* size) diff --git a/src/unix/pipe.c b/src/unix/pipe.c index b73994cb..80a8d90a 100644 --- a/src/unix/pipe.c +++ b/src/unix/pipe.c @@ -45,9 +45,14 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) { const char* pipe_fname; int sockfd; int err; + size_t name_len; pipe_fname = NULL; sockfd = -1; + name_len = strlen(name); + + if (name_len > sizeof(saddr.sun_path) - 1) + return -ENAMETOOLONG; /* Already bound? */ if (uv__stream_fd(handle) >= 0) @@ -67,8 +72,7 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) { sockfd = err; memset(&saddr, 0, sizeof saddr); - strncpy(saddr.sun_path, pipe_fname, sizeof(saddr.sun_path) - 1); - saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0'; + memcpy(saddr.sun_path, pipe_fname, name_len); saddr.sun_family = AF_UNIX; if (bind(sockfd, (struct sockaddr*)&saddr, sizeof saddr)) { @@ -160,6 +164,14 @@ void uv_pipe_connect(uv_connect_t* req, int new_sock; int err; int r; + size_t name_len; + + name_len = strlen(name); + + if (name_len > sizeof(saddr.sun_path) - 1) { + err = -ENAMETOOLONG; + goto out; + } new_sock = (uv__stream_fd(handle) == -1); @@ -171,8 +183,7 @@ void uv_pipe_connect(uv_connect_t* req, } memset(&saddr, 0, sizeof saddr); - strncpy(saddr.sun_path, name, sizeof(saddr.sun_path) - 1); - saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0'; + memcpy(saddr.sun_path, name, name_len); saddr.sun_family = AF_UNIX; do { diff --git a/test/test-list.h b/test/test-list.h index 52a5970c..992ea512 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -135,10 +135,16 @@ TEST_DECLARE (udp_try_send) TEST_DECLARE (pipe_bind_error_addrinuse) TEST_DECLARE (pipe_bind_error_addrnotavail) TEST_DECLARE (pipe_bind_error_inval) +#ifndef _WIN32 +TEST_DECLARE (pipe_bind_error_long_path) +#endif TEST_DECLARE (pipe_connect_multiple) TEST_DECLARE (pipe_listen_without_bind) TEST_DECLARE (pipe_connect_bad_name) TEST_DECLARE (pipe_connect_to_file) +#ifndef _WIN32 +TEST_DECLARE (pipe_connect_to_long_path) +#endif TEST_DECLARE (pipe_connect_on_prepare) TEST_DECLARE (pipe_getsockname) TEST_DECLARE (pipe_getsockname_abstract) @@ -401,6 +407,9 @@ TASK_LIST_START TEST_ENTRY (pipe_connect_bad_name) TEST_ENTRY (pipe_connect_to_file) +#ifndef _WIN32 + TEST_ENTRY (pipe_connect_to_long_path) +#endif TEST_ENTRY (pipe_connect_on_prepare) TEST_ENTRY (pipe_server_close) @@ -526,6 +535,9 @@ TASK_LIST_START TEST_ENTRY (pipe_bind_error_addrinuse) TEST_ENTRY (pipe_bind_error_addrnotavail) TEST_ENTRY (pipe_bind_error_inval) +#ifndef _WIN32 + TEST_ENTRY (pipe_bind_error_long_path) +#endif TEST_ENTRY (pipe_connect_multiple) TEST_ENTRY (pipe_listen_without_bind) TEST_ENTRY (pipe_getsockname) diff --git a/test/test-pipe-bind-error.c b/test/test-pipe-bind-error.c index 38b57db6..b2272e25 100644 --- a/test/test-pipe-bind-error.c +++ b/test/test-pipe-bind-error.c @@ -23,6 +23,7 @@ #include "task.h" #include #include +#include #ifdef _WIN32 @@ -134,3 +135,27 @@ TEST_IMPL(pipe_listen_without_bind) { MAKE_VALGRIND_HAPPY(); return 0; } + + +TEST_IMPL(pipe_bind_error_long_path) { + char path[2048]; + uv_pipe_t server; + int r; + + memset(path, '.', sizeof(path) - 1); + path[sizeof(path) - 1] = '\0'; + + r = uv_pipe_init(uv_default_loop(), &server, 0); + ASSERT(r == 0); + r = uv_pipe_bind(&server, path); + ASSERT(r == UV_ENAMETOOLONG); + + uv_close((uv_handle_t*)&server, close_cb); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + ASSERT(close_cb_called == 1); + + MAKE_VALGRIND_HAPPY(); + return 0; +} diff --git a/test/test-pipe-connect-error.c b/test/test-pipe-connect-error.c index ebb2a6ca..bf1896a8 100644 --- a/test/test-pipe-connect-error.c +++ b/test/test-pipe-connect-error.c @@ -23,6 +23,7 @@ #include "task.h" #include #include +#include #ifdef _WIN32 @@ -56,6 +57,13 @@ static void connect_cb_file(uv_connect_t* connect_req, int status) { } +static void connect_cb_long_path(uv_connect_t* connect_req, int status) { + ASSERT(status == UV_ENAMETOOLONG); + uv_close((uv_handle_t*)connect_req->handle, close_cb); + connect_cb_called++; +} + + TEST_IMPL(pipe_connect_bad_name) { uv_pipe_t client; uv_connect_t req; @@ -93,3 +101,26 @@ TEST_IMPL(pipe_connect_to_file) { MAKE_VALGRIND_HAPPY(); return 0; } + + +TEST_IMPL(pipe_connect_to_long_path) { + char path[2048]; + uv_pipe_t client; + uv_connect_t req; + int r; + + memset(path, '.', sizeof(path) - 1); + path[sizeof(path) - 1] = '\0'; + + r = uv_pipe_init(uv_default_loop(), &client, 0); + ASSERT(r == 0); + uv_pipe_connect(&req, &client, path, connect_cb_long_path); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + ASSERT(close_cb_called == 1); + ASSERT(connect_cb_called == 1); + + MAKE_VALGRIND_HAPPY(); + return 0; +}