unix,win: fix read past end of pipe name buffer (#4209)

Passing a socket name without a trailing nul byte to uv_pipe_bind2() or
(on Windows) uv_pipe_connect2() resulted in reading beyond the end of
the name buffer when copying or converting it.

Fix that by copying the socket name to temporary storage first and add
the trailing nul byte explicitly.

Add a check for embedded nul bytes in the socket name.

Fix a small memory leak in the Windows error path of uv_pipe_bind2().
This commit is contained in:
Ben Noordhuis 2023-11-16 09:05:51 +01:00 committed by GitHub
parent f144429365
commit 6be130e1b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 96 additions and 25 deletions

View File

@ -30,6 +30,19 @@
#include <stdlib.h>
/* Does the file path contain embedded nul bytes? */
static int includes_nul(const char *s, size_t n) {
if (n == 0)
return 0;
#ifdef __linux__
/* Accept abstract socket namespace path ("\0/virtual/path"). */
s++;
n--;
#endif
return NULL != memchr(s, '\0', n);
}
int uv_pipe_init(uv_loop_t* loop, uv_pipe_t* handle, int ipc) {
uv__stream_init(loop, (uv_stream_t*)handle, UV_NAMED_PIPE);
handle->shutdown_req = NULL;
@ -65,11 +78,8 @@ int uv_pipe_bind2(uv_pipe_t* handle,
if (namelen == 0)
return UV_EINVAL;
#ifndef __linux__
/* Abstract socket namespace only works on Linux. */
if (*name == '\0')
if (includes_nul(name, namelen))
return UV_EINVAL;
#endif
if (flags & UV_PIPE_NO_TRUNCATE)
if (namelen > sizeof(saddr.sun_path))
@ -91,9 +101,11 @@ int uv_pipe_bind2(uv_pipe_t* handle,
* automatically since they're not real file system entities.
*/
if (*name != '\0') {
pipe_fname = uv__strdup(name);
pipe_fname = uv__malloc(namelen + 1);
if (pipe_fname == NULL)
return UV_ENOMEM;
memcpy(pipe_fname, name, namelen);
pipe_fname[namelen] = '\0';
}
err = uv__socket(AF_UNIX, SOCK_STREAM, 0);
@ -117,7 +129,7 @@ int uv_pipe_bind2(uv_pipe_t* handle,
/* Success. */
handle->flags |= UV_HANDLE_BOUND;
handle->pipe_fname = pipe_fname; /* NULL or a strdup'ed copy. */
handle->pipe_fname = pipe_fname; /* NULL or a copy of |name| */
handle->io_watcher.fd = sockfd;
return 0;
@ -249,11 +261,8 @@ int uv_pipe_connect2(uv_connect_t* req,
if (namelen == 0)
return UV_EINVAL;
#ifndef __linux__
/* Abstract socket namespace only works on Linux. */
if (*name == '\0')
if (includes_nul(name, namelen))
return UV_EINVAL;
#endif
if (flags & UV_PIPE_NO_TRUNCATE)
if (namelen > sizeof(saddr.sun_path))

View File

@ -98,6 +98,14 @@ static void eof_timer_destroy(uv_pipe_t* pipe);
static void eof_timer_close_cb(uv_handle_t* handle);
/* Does the file path contain embedded nul bytes? */
static int includes_nul(const char *s, size_t n) {
if (n == 0)
return 0;
return NULL != memchr(s, '\0', n);
}
static void uv__unique_pipe_name(char* ptr, char* name, size_t size) {
snprintf(name, size, "\\\\?\\pipe\\uv\\%p-%lu", ptr, GetCurrentProcessId());
}
@ -705,6 +713,7 @@ int uv_pipe_bind2(uv_pipe_t* handle,
uv_loop_t* loop = handle->loop;
int i, err;
uv_pipe_accept_t* req;
char* name_copy;
if (flags & ~UV_PIPE_NO_TRUNCATE) {
return UV_EINVAL;
@ -718,7 +727,7 @@ int uv_pipe_bind2(uv_pipe_t* handle,
return UV_EINVAL;
}
if (*name == '\0') {
if (includes_nul(name, namelen)) {
return UV_EINVAL;
}
@ -730,14 +739,24 @@ int uv_pipe_bind2(uv_pipe_t* handle,
return UV_EINVAL;
}
name_copy = uv__malloc(namelen + 1);
if (name_copy == NULL) {
return UV_ENOMEM;
}
memcpy(name_copy, name, namelen);
name_copy[namelen] = '\0';
if (!(handle->flags & UV_HANDLE_PIPESERVER)) {
handle->pipe.serv.pending_instances = default_pending_pipe_instances;
}
err = UV_ENOMEM;
handle->pipe.serv.accept_reqs = (uv_pipe_accept_t*)
uv__malloc(sizeof(uv_pipe_accept_t) * handle->pipe.serv.pending_instances);
if (!handle->pipe.serv.accept_reqs)
return UV_ENOMEM;
if (handle->pipe.serv.accept_reqs == NULL) {
goto error;
}
for (i = 0; i < handle->pipe.serv.pending_instances; i++) {
req = &handle->pipe.serv.accept_reqs[i];
@ -747,9 +766,14 @@ int uv_pipe_bind2(uv_pipe_t* handle,
req->next_pending = NULL;
}
err = uv__convert_utf8_to_utf16(name, &handle->name);
if (err)
return err;
/* TODO(bnoordhuis) Add converters that take a |length| parameter. */
err = uv__convert_utf8_to_utf16(name_copy, &handle->name);
uv__free(name_copy);
name_copy = NULL;
if (err) {
goto error;
}
/*
* Attempt to create the first pipe with FILE_FLAG_FIRST_PIPE_INSTANCE.
@ -761,9 +785,11 @@ int uv_pipe_bind2(uv_pipe_t* handle,
TRUE)) {
err = GetLastError();
if (err == ERROR_ACCESS_DENIED) {
err = WSAEADDRINUSE; /* Translates to UV_EADDRINUSE. */
err = UV_EADDRINUSE;
} else if (err == ERROR_PATH_NOT_FOUND || err == ERROR_INVALID_NAME) {
err = WSAEACCES; /* Translates to UV_EACCES. */
err = UV_EACCES;
} else {
err = uv_translate_sys_error(err);
}
goto error;
}
@ -775,10 +801,13 @@ int uv_pipe_bind2(uv_pipe_t* handle,
return 0;
error:
uv__free(handle->pipe.serv.accept_reqs);
uv__free(handle->name);
uv__free(name_copy);
handle->pipe.serv.accept_reqs = NULL;
handle->name = NULL;
return uv_translate_sys_error(err);
return err;
}
@ -855,6 +884,7 @@ int uv_pipe_connect2(uv_connect_t* req,
size_t nameSize;
HANDLE pipeHandle = INVALID_HANDLE_VALUE;
DWORD duplex_flags;
char* name_copy;
loop = handle->loop;
UV_REQ_INIT(req, UV_CONNECT);
@ -876,10 +906,18 @@ int uv_pipe_connect2(uv_connect_t* req,
return UV_EINVAL;
}
if (*name == '\0') {
if (includes_nul(name, namelen)) {
return UV_EINVAL;
}
name_copy = uv__malloc(namelen + 1);
if (name_copy == NULL) {
return UV_ENOMEM;
}
memcpy(name_copy, name, namelen);
name_copy[namelen] = '\0';
if (handle->flags & UV_HANDLE_PIPESERVER) {
err = ERROR_INVALID_PARAMETER;
goto error;
@ -890,7 +928,11 @@ int uv_pipe_connect2(uv_connect_t* req,
}
uv__pipe_connection_init(handle);
err = uv__convert_utf8_to_utf16(name, &handle->name);
/* TODO(bnoordhuis) Add converters that take a |length| parameter. */
err = uv__convert_utf8_to_utf16(name_copy, &handle->name);
uv__free(name_copy);
name_copy = NULL;
if (err) {
err = ERROR_NO_UNICODE_TRANSLATION;
goto error;
@ -936,6 +978,8 @@ int uv_pipe_connect2(uv_connect_t* req,
return 0;
error:
uv__free(name_copy);
if (handle->name) {
uv__free(handle->name);
handle->name = NULL;

View File

@ -91,16 +91,24 @@ TEST_IMPL(pipe_getsockname) {
RETURN_SKIP(NO_SELF_CONNECT);
#endif
uv_loop_t* loop;
char namebuf[256];
char buf[1024];
size_t namelen;
size_t len;
int r;
snprintf(namebuf, sizeof(namebuf), "%s-oob", TEST_PIPENAME);
namelen = sizeof(TEST_PIPENAME) - 1;
loop = uv_default_loop();
ASSERT_NOT_NULL(loop);
r = uv_pipe_init(loop, &pipe_server, 0);
ASSERT_OK(r);
r = uv_pipe_bind2(&pipe_server, "bad\0path", 8, 0);
ASSERT_EQ(r, UV_EINVAL);
len = sizeof buf;
r = uv_pipe_getsockname(&pipe_server, buf, &len);
ASSERT_EQ(r, UV_EBADF);
@ -109,9 +117,13 @@ TEST_IMPL(pipe_getsockname) {
r = uv_pipe_getpeername(&pipe_server, buf, &len);
ASSERT_EQ(r, UV_EBADF);
r = uv_pipe_bind(&pipe_server, TEST_PIPENAME);
r = uv_pipe_bind2(&pipe_server, namebuf, namelen, 0);
ASSERT_OK(r);
#ifndef _WIN32
ASSERT_STR_EQ(pipe_server.pipe_fname, TEST_PIPENAME);
#endif
len = sizeof buf;
r = uv_pipe_getsockname(&pipe_server, buf, &len);
ASSERT_OK(r);
@ -138,7 +150,13 @@ TEST_IMPL(pipe_getsockname) {
r = uv_pipe_getpeername(&pipe_client, buf, &len);
ASSERT_EQ(r, UV_EBADF);
uv_pipe_connect(&connect_req, &pipe_client, TEST_PIPENAME, pipe_client_connect_cb);
r = uv_pipe_connect2(&connect_req,
&pipe_client,
namebuf,
namelen,
0,
pipe_client_connect_cb);
ASSERT_OK(r);
len = sizeof buf;
r = uv_pipe_getsockname(&pipe_client, buf, &len);
@ -171,7 +189,7 @@ TEST_IMPL(pipe_getsockname_abstract) {
buflen = sizeof(buf);
memset(buf, 0, sizeof(buf));
ASSERT_OK(uv_pipe_init(uv_default_loop(), &pipe_server, 0));
ASSERT_OK(uv_pipe_bind2(&pipe_server, name, sizeof(name), 0));
ASSERT_OK(uv_pipe_bind2(&pipe_server, name, sizeof(name) - 1, 0));
ASSERT_OK(uv_pipe_getsockname(&pipe_server, buf, &buflen));
ASSERT_MEM_EQ(name, buf, sizeof(name));
ASSERT_OK(uv_listen((uv_stream_t*) &pipe_server,
@ -181,7 +199,7 @@ TEST_IMPL(pipe_getsockname_abstract) {
ASSERT_OK(uv_pipe_connect2(&connect_req,
&pipe_client,
name,
sizeof(name),
sizeof(name) - 1,
0,
pipe_client_connect_cb));
ASSERT_OK(uv_run(uv_default_loop(), UV_RUN_DEFAULT));