From 840a8c599ec1f138992a538f5b8109f91f31805e Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 27 Jun 2016 16:09:46 -0400 Subject: [PATCH] unix,win: make uv_get_process_title() stricter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit causes uv_get_process_title() to: - return EINVAL if the buffer is null or size is 0 - return ENOBUFS if the title is too big for the buffer - null terminate the buffer on success Fixes: https://github.com/libuv/libuv/issues/315 PR-URL: https://github.com/libuv/libuv/pull/928 Reviewed-By: Saúl Ibarra Corretgé --- docs/src/misc.rst | 4 +++- src/unix/aix.c | 27 ++++++++++++++------------- src/unix/freebsd.c | 18 ++++++++++++++---- src/unix/netbsd.c | 18 ++++++++++++++---- src/unix/openbsd.c | 18 ++++++++++++++---- src/unix/proctitle.c | 11 +++++++---- src/unix/sunos.c | 7 ++++--- src/win/util.c | 14 +++++++++++++- test/test-process-title.c | 22 ++++++++++++++++++++++ 9 files changed, 105 insertions(+), 34 deletions(-) diff --git a/docs/src/misc.rst b/docs/src/misc.rst index f32af48ff..3b7f31aac 100644 --- a/docs/src/misc.rst +++ b/docs/src/misc.rst @@ -183,7 +183,9 @@ API .. c:function:: int uv_get_process_title(char* buffer, size_t size) - Gets the title of the current process. + Gets the title of the current process. If `buffer` is `NULL` or `size` is + zero, `UV_EINVAL` is returned. If `size` cannot accommodate the process + title and terminating `NULL` character, the function returns `UV_ENOBUFS`. .. c:function:: int uv_set_process_title(const char* title) diff --git a/src/unix/aix.c b/src/unix/aix.c index 2276985fc..652cd980f 100644 --- a/src/unix/aix.c +++ b/src/unix/aix.c @@ -156,7 +156,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { pqry.fd = pc.fd; rc = pollset_query(loop->backend_fd, &pqry); switch (rc) { - case -1: + case -1: assert(0 && "Failed to query pollset for file descriptor"); abort(); case 0: @@ -333,20 +333,20 @@ int uv_exepath(char* buffer, size_t* size) { pi.pi_pid = getpid(); res = getargs(&pi, sizeof(pi), args, sizeof(args)); - if (res < 0) + if (res < 0) return -EINVAL; /* * Possibilities for args: * i) an absolute path such as: /home/user/myprojects/nodejs/node * ii) a relative path such as: ./node or ../myprojects/nodejs/node - * iii) a bare filename such as "node", after exporting PATH variable + * iii) a bare filename such as "node", after exporting PATH variable * to its location. */ /* Case i) and ii) absolute or relative paths */ if (strchr(args, '/') != NULL) { - if (realpath(args, abspath) != abspath) + if (realpath(args, abspath) != abspath) return -errno; abspath_size = strlen(abspath); @@ -360,7 +360,7 @@ int uv_exepath(char* buffer, size_t* size) { return 0; } else { - /* Case iii). Search PATH environment variable */ + /* Case iii). Search PATH environment variable */ char trypath[PATH_MAX]; char *clonedpath = NULL; char *token = NULL; @@ -376,7 +376,7 @@ int uv_exepath(char* buffer, size_t* size) { token = strtok(clonedpath, ":"); while (token != NULL) { snprintf(trypath, sizeof(trypath) - 1, "%s/%s", token, args); - if (realpath(trypath, abspath) == abspath) { + if (realpath(trypath, abspath) == abspath) { /* Check the match is executable */ if (access(abspath, X_OK) == 0) { abspath_size = strlen(abspath); @@ -452,7 +452,7 @@ static char *uv__rawname(char *cp) { } -/* +/* * Determine whether given pathname is a directory * Returns 0 if the path is a directory, -1 if not * @@ -472,7 +472,7 @@ static int uv__path_is_a_directory(char* filename) { } -/* +/* * Check whether AHAFS is mounted. * Returns 0 if AHAFS is mounted, or an error code < 0 on failure */ @@ -547,7 +547,7 @@ static int uv__makedir_p(const char *dir) { return mkdir(tmp, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); } -/* +/* * Creates necessary subdirectories in the AIX Event Infrastructure * file system for monitoring the object specified. * Returns code from mkdir call @@ -665,7 +665,7 @@ static int uv__skip_lines(char **p, int n) { /* * Parse the event occurrence data to figure out what event just occurred * and take proper action. - * + * * The buf is a pointer to the buffer containing the event occurrence data * Returns 0 on success, -1 if unrecoverable error in parsing * @@ -891,9 +891,10 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { - if (size > 0) { - buffer[0] = '\0'; - } + if (buffer == NULL || size == 0) + return -EINVAL; + + buffer[0] = '\0'; return 0; } diff --git a/src/unix/freebsd.c b/src/unix/freebsd.c index adc95235c..cba44a3e0 100644 --- a/src/unix/freebsd.c +++ b/src/unix/freebsd.c @@ -196,14 +196,24 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { + size_t len; + + if (buffer == NULL || size == 0) + return -EINVAL; + if (process_title) { - strncpy(buffer, process_title, size); + len = strlen(process_title) + 1; + + if (size < len) + return -ENOBUFS; + + memcpy(buffer, process_title, len); } else { - if (size > 0) { - buffer[0] = '\0'; - } + len = 0; } + buffer[len] = '\0'; + return 0; } diff --git a/src/unix/netbsd.c b/src/unix/netbsd.c index ca48550f9..4a9e6cbc1 100644 --- a/src/unix/netbsd.c +++ b/src/unix/netbsd.c @@ -147,14 +147,24 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { + size_t len; + + if (buffer == NULL || size == 0) + return -EINVAL; + if (process_title) { - strncpy(buffer, process_title, size); + len = strlen(process_title) + 1; + + if (size < len) + return -ENOBUFS; + + memcpy(buffer, process_title, len); } else { - if (size > 0) { - buffer[0] = '\0'; - } + len = 0; } + buffer[len] = '\0'; + return 0; } diff --git a/src/unix/openbsd.c b/src/unix/openbsd.c index 8c40bde40..909288cc8 100644 --- a/src/unix/openbsd.c +++ b/src/unix/openbsd.c @@ -169,14 +169,24 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { + size_t len; + + if (buffer == NULL || size == 0) + return -EINVAL; + if (process_title) { - strncpy(buffer, process_title, size); + len = strlen(process_title) + 1; + + if (size < len) + return -ENOBUFS; + + memcpy(buffer, process_title, len); } else { - if (size > 0) { - buffer[0] = '\0'; - } + len = 0; } + buffer[len] = '\0'; + return 0; } diff --git a/src/unix/proctitle.c b/src/unix/proctitle.c index 19214e5ec..08d875f7a 100644 --- a/src/unix/proctitle.c +++ b/src/unix/proctitle.c @@ -87,10 +87,13 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { - if (process_title.len > 0) - strncpy(buffer, process_title.str, size); - else if (size > 0) - buffer[0] = '\0'; + if (buffer == NULL || size == 0) + return -EINVAL; + else if (size <= process_title.len) + return -ENOBUFS; + + memcpy(buffer, process_title.str, process_title.len + 1); + buffer[process_title.len] = '\0'; return 0; } diff --git a/src/unix/sunos.c b/src/unix/sunos.c index efadcea72..3e7a7592d 100644 --- a/src/unix/sunos.c +++ b/src/unix/sunos.c @@ -542,9 +542,10 @@ int uv_set_process_title(const char* title) { int uv_get_process_title(char* buffer, size_t size) { - if (size > 0) { - buffer[0] = '\0'; - } + if (buffer == NULL || size == 0) + return -EINVAL; + + buffer[0] = '\0'; return 0; } diff --git a/src/win/util.c b/src/win/util.c index 4cebad390..84a0e467e 100644 --- a/src/win/util.c +++ b/src/win/util.c @@ -416,6 +416,11 @@ static int uv__get_process_title() { int uv_get_process_title(char* buffer, size_t size) { + size_t len; + + if (buffer == NULL || size == 0) + return UV_EINVAL; + uv__once_init(); EnterCriticalSection(&process_title_lock); @@ -429,7 +434,14 @@ int uv_get_process_title(char* buffer, size_t size) { } assert(process_title); - strncpy(buffer, process_title, size); + len = strlen(process_title) + 1; + + if (size < len) { + LeaveCriticalSection(&process_title_lock); + return UV_ENOBUFS; + } + + memcpy(buffer, process_title, len); LeaveCriticalSection(&process_title_lock); return 0; diff --git a/test/test-process-title.c b/test/test-process-title.c index 42ade4416..00f164a41 100644 --- a/test/test-process-title.c +++ b/test/test-process-title.c @@ -41,6 +41,24 @@ static void set_title(const char* title) { } +static void uv_get_process_title_edge_cases() { + char buffer[512]; + int r; + + /* Test a NULL buffer */ + r = uv_get_process_title(NULL, 100); + ASSERT(r == UV_EINVAL); + + /* Test size of zero */ + r = uv_get_process_title(buffer, 0); + ASSERT(r == UV_EINVAL); + + /* Test for insufficient buffer size */ + r = uv_get_process_title(buffer, 1); + ASSERT(r == UV_ENOBUFS); +} + + TEST_IMPL(process_title) { #if defined(__sun) || defined(_AIX) RETURN_SKIP("uv_(get|set)_process_title is not implemented."); @@ -48,6 +66,10 @@ TEST_IMPL(process_title) { /* Check for format string vulnerabilities. */ set_title("%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s"); set_title("new title"); + + /* Check uv_get_process_title() edge cases */ + uv_get_process_title_edge_cases(); + return 0; #endif }