Skip to content

Commit

Permalink
unix: fail with ENAMETOOLONG in uv_pipe_*
Browse files Browse the repository at this point in the history
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.

Ref: nodejs/node#12601
Ref: nodejs/node#12708
  • Loading branch information
addaleax committed Apr 27, 2017
1 parent adedc27 commit 050f0f7
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 4 deletions.
19 changes: 15 additions & 4 deletions src/unix/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) + 1;

if (name_len > sizeof(saddr.sun_path))
return -ENAMETOOLONG;

/* Already bound? */
if (uv__stream_fd(handle) >= 0)
Expand All @@ -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)) {
Expand Down Expand Up @@ -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) + 1;

if (name_len > sizeof(saddr.sun_path)) {
err = -ENAMETOOLONG;
goto out;
}

new_sock = (uv__stream_fd(handle) == -1);

Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,12 @@ TEST_DECLARE (udp_try_send)
TEST_DECLARE (pipe_bind_error_addrinuse)
TEST_DECLARE (pipe_bind_error_addrnotavail)
TEST_DECLARE (pipe_bind_error_inval)
TEST_DECLARE (pipe_bind_error_long_path)
TEST_DECLARE (pipe_connect_multiple)
TEST_DECLARE (pipe_listen_without_bind)
TEST_DECLARE (pipe_connect_bad_name)
TEST_DECLARE (pipe_connect_to_file)
TEST_DECLARE (pipe_connect_to_long_path)
TEST_DECLARE (pipe_connect_on_prepare)
TEST_DECLARE (pipe_getsockname)
TEST_DECLARE (pipe_getsockname_abstract)
Expand Down Expand Up @@ -398,6 +400,7 @@ TASK_LIST_START

TEST_ENTRY (pipe_connect_bad_name)
TEST_ENTRY (pipe_connect_to_file)
TEST_ENTRY (pipe_connect_to_long_path)
TEST_ENTRY (pipe_connect_on_prepare)

TEST_ENTRY (pipe_server_close)
Expand Down Expand Up @@ -523,6 +526,7 @@ TASK_LIST_START
TEST_ENTRY (pipe_bind_error_addrinuse)
TEST_ENTRY (pipe_bind_error_addrnotavail)
TEST_ENTRY (pipe_bind_error_inval)
TEST_ENTRY (pipe_bind_error_long_path)
TEST_ENTRY (pipe_connect_multiple)
TEST_ENTRY (pipe_listen_without_bind)
TEST_ENTRY (pipe_getsockname)
Expand Down
25 changes: 25 additions & 0 deletions test/test-pipe-bind-error.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "task.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


#ifdef _WIN32
Expand Down Expand Up @@ -134,3 +135,27 @@ TEST_IMPL(pipe_listen_without_bind) {
MAKE_VALGRIND_HAPPY();
return 0;
}


TEST_IMPL(pipe_bind_error_long_path) {
char path[256];
uv_pipe_t server;
int r;

memset(path, '.', 255);
path[255] = '\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;
}
31 changes: 31 additions & 0 deletions test/test-pipe-connect-error.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "task.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


#ifdef _WIN32
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -93,3 +101,26 @@ TEST_IMPL(pipe_connect_to_file) {
MAKE_VALGRIND_HAPPY();
return 0;
}


TEST_IMPL(pipe_connect_to_long_path) {
char path[256];
uv_pipe_t client;
uv_connect_t req;
int r;

memset(path, '.', 255);
path[255] = '\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;
}

0 comments on commit 050f0f7

Please sign in to comment.