From e208100fc9ef14e775927ccb99e934b2007e671f Mon Sep 17 00:00:00 2001 From: tjarlama <59913901+tjarlama@users.noreply.github.com> Date: Wed, 12 Aug 2020 09:57:26 +0530 Subject: [PATCH] fs: clobber req->path on uv_fs_mkstemp() error Contents of template variable passed for posix call mkstemp on error code EINVAL is unknown. On AIX platform, template will get clobbered on EINVAL and any attempt to read template might result in error. In libuv, req->path is passed directly to the mkstemp call and behavior of this string on error is platform dependent. To avoid portability issues, it's better to have a common behavior on all platform. For both unix and windows platform libuv will rewrite path with an empty string on all error cases. Fixes: https://github.com/libuv/libuv/issues/2913 Refs: https://github.com/nodejs/node/pull/33549 Refs: https://github.com/libuv/libuv/pull/2933 PR-URL: https://github.com/libuv/libuv/pull/2938 Reviewed-By: Jameson Nash --- src/unix/fs.c | 8 ++++++-- src/win/fs.c | 21 ++++++++++++--------- test/test-fs.c | 5 ++++- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/unix/fs.c b/src/unix/fs.c index cbf078b7..87cb8b81 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -306,7 +306,8 @@ static int uv__fs_mkstemp(uv_fs_t* req) { if (path_length < pattern_size || strcmp(path + path_length - pattern_size, pattern)) { errno = EINVAL; - return -1; + r = -1; + goto clobber; } uv_once(&once, uv__mkostemp_initonce); @@ -321,7 +322,7 @@ static int uv__fs_mkstemp(uv_fs_t* req) { /* If mkostemp() returns EINVAL, it means the kernel doesn't support O_CLOEXEC, so we just fallback to mkstemp() below. */ if (errno != EINVAL) - return r; + goto clobber; /* We set the static variable so that next calls don't even try to use mkostemp. */ @@ -347,6 +348,9 @@ static int uv__fs_mkstemp(uv_fs_t* req) { if (req->cb != NULL) uv_rwlock_rdunlock(&req->loop->cloexec_lock); +clobber: + if (r < 0) + path[0] = '\0'; return r; } diff --git a/src/win/fs.c b/src/win/fs.c index 0172346b..8a801749 100644 --- a/src/win/fs.c +++ b/src/win/fs.c @@ -1241,19 +1241,21 @@ void fs__mktemp(uv_fs_t* req, uv__fs_mktemp_func func) { unsigned int tries, i; size_t len; uint64_t v; - + char* path; + + path = req->path; len = wcslen(req->file.pathw); ep = req->file.pathw + len; if (len < num_x || wcsncmp(ep - num_x, L"XXXXXX", num_x)) { SET_REQ_UV_ERROR(req, UV_EINVAL, ERROR_INVALID_PARAMETER); - return; + goto clobber; } tries = TMP_MAX; do { if (uv__random_rtlgenrandom((void *)&v, sizeof(v)) < 0) { SET_REQ_UV_ERROR(req, UV_EIO, ERROR_IO_DEVICE); - break; + goto clobber; } cp = ep - num_x; @@ -1264,16 +1266,17 @@ void fs__mktemp(uv_fs_t* req, uv__fs_mktemp_func func) { if (func(req)) { if (req->result >= 0) { - len = strlen(req->path); - wcstombs((char*) req->path + len - num_x, ep - num_x, num_x); + len = strlen(path); + wcstombs(path + len - num_x, ep - num_x, num_x); } - break; + return; } } while (--tries); - if (tries == 0) { - SET_REQ_WIN32_ERROR(req, GetLastError()); - } + SET_REQ_WIN32_ERROR(req, GetLastError()); + +clobber: + path[0] = '\0'; } diff --git a/test/test-fs.c b/test/test-fs.c index ae9923a9..656cd3ad 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -1290,7 +1290,10 @@ TEST_IMPL(fs_mkstemp) { ASSERT(strcmp(mkstemp_req1.path, mkstemp_req2.path) != 0); /* invalid template returns EINVAL */ - ASSERT(uv_fs_mkstemp(NULL, &mkstemp_req3, "test_file", NULL) == UV_EINVAL); + ASSERT_EQ(UV_EINVAL, uv_fs_mkstemp(NULL, &mkstemp_req3, "test_file", NULL)); + + /* Make sure that path is empty string */ + ASSERT_EQ(0, strlen(mkstemp_req3.path)); /* We can write to the opened file */ iov = uv_buf_init(test_buf, sizeof(test_buf));