unix,win: check nbufs argument is reasonable (#5014)
Catch sign conversion bugs by introducing a bounds check that doubles as a sanity check. Fixes: https://github.com/libuv/libuv/issues/5012
This commit is contained in:
parent
4839e28d50
commit
91dc2389cc
@ -1297,12 +1297,18 @@ static void uv__stream_connect(uv_stream_t* stream) {
|
||||
static int uv__check_before_write(uv_stream_t* stream,
|
||||
unsigned int nbufs,
|
||||
uv_stream_t* send_handle) {
|
||||
assert(nbufs > 0);
|
||||
assert((stream->type == UV_TCP ||
|
||||
stream->type == UV_NAMED_PIPE ||
|
||||
stream->type == UV_TTY) &&
|
||||
"uv_write (unix) does not yet support other types of streams");
|
||||
|
||||
/* We're not beholden to IOV_MAX but limit the buffer count to catch sign
|
||||
* conversion bugs where a caller passes in a signed negative number that
|
||||
* then gets converted to a really large unsigned number.
|
||||
*/
|
||||
if (nbufs < 1 || nbufs > 1024*1024)
|
||||
return UV_EINVAL;
|
||||
|
||||
if (uv__stream_fd(stream) < 0)
|
||||
return UV_EBADF;
|
||||
|
||||
|
||||
@ -111,6 +111,23 @@ int uv_read_stop(uv_stream_t* handle) {
|
||||
}
|
||||
|
||||
|
||||
static int uv__check_before_write(uv_stream_t* handle, unsigned int nbufs) {
|
||||
/* We're not beholden to IOV_MAX but limit the buffer count to catch sign
|
||||
* conversion bugs where a caller passes in a signed negative number that
|
||||
* then gets converted to a really large unsigned number.
|
||||
*/
|
||||
if (nbufs < 1 || nbufs > 1024*1024) {
|
||||
return UV_EINVAL;
|
||||
}
|
||||
|
||||
if (!(handle->flags & UV_HANDLE_WRITABLE)) {
|
||||
return UV_EPIPE;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
int uv_write(uv_write_t* req,
|
||||
uv_stream_t* handle,
|
||||
const uv_buf_t bufs[],
|
||||
@ -119,8 +136,9 @@ int uv_write(uv_write_t* req,
|
||||
uv_loop_t* loop = handle->loop;
|
||||
int err;
|
||||
|
||||
if (!(handle->flags & UV_HANDLE_WRITABLE)) {
|
||||
return UV_EPIPE;
|
||||
err = uv__check_before_write(handle, nbufs);
|
||||
if (err != 0) {
|
||||
return err;
|
||||
}
|
||||
|
||||
err = ERROR_INVALID_PARAMETER;
|
||||
@ -156,10 +174,13 @@ int uv_write2(uv_write_t* req,
|
||||
return uv_write(req, handle, bufs, nbufs, cb);
|
||||
}
|
||||
|
||||
err = uv__check_before_write(handle, nbufs);
|
||||
if (err != 0) {
|
||||
return err;
|
||||
}
|
||||
|
||||
if (handle->type != UV_NAMED_PIPE || !((uv_pipe_t*) handle)->ipc) {
|
||||
return UV_EINVAL;
|
||||
} else if (!(handle->flags & UV_HANDLE_WRITABLE)) {
|
||||
return UV_EPIPE;
|
||||
}
|
||||
|
||||
err = uv__pipe_write(
|
||||
@ -171,10 +192,15 @@ int uv_write2(uv_write_t* req,
|
||||
int uv_try_write(uv_stream_t* stream,
|
||||
const uv_buf_t bufs[],
|
||||
unsigned int nbufs) {
|
||||
int err;
|
||||
|
||||
err = uv__check_before_write(stream, nbufs);
|
||||
if (err != 0) {
|
||||
return err;
|
||||
}
|
||||
|
||||
if (stream->flags & UV_HANDLE_CLOSING)
|
||||
return UV_EBADF;
|
||||
if (!(stream->flags & UV_HANDLE_WRITABLE))
|
||||
return UV_EPIPE;
|
||||
|
||||
switch (stream->type) {
|
||||
case UV_TCP:
|
||||
|
||||
@ -91,6 +91,7 @@ static void connect_cb(uv_connect_t* req, int status) {
|
||||
TEST_IMPL(tcp_write_fail) {
|
||||
struct sockaddr_in addr;
|
||||
uv_tcp_t client;
|
||||
uv_buf_t buf;
|
||||
int r;
|
||||
|
||||
ASSERT_OK(uv_ip4_addr("127.0.0.1", TEST_PORT, &addr));
|
||||
@ -98,6 +99,20 @@ TEST_IMPL(tcp_write_fail) {
|
||||
r = uv_tcp_init(uv_default_loop(), &client);
|
||||
ASSERT_OK(r);
|
||||
|
||||
r = uv_write(&write_req,
|
||||
(uv_stream_t*) &client,
|
||||
&buf,
|
||||
0, /* Illegal. Worse, senseless. */
|
||||
write_cb);
|
||||
ASSERT_EQ(UV_EINVAL, r);
|
||||
|
||||
r = uv_write(&write_req,
|
||||
(uv_stream_t*) &client,
|
||||
&buf,
|
||||
-42, /* Undergoes sign conversion. */
|
||||
write_cb);
|
||||
ASSERT_EQ(UV_EINVAL, r);
|
||||
|
||||
r = uv_tcp_connect(&connect_req,
|
||||
&client,
|
||||
(const struct sockaddr*) &addr,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user