unix,win: make uv_get_process_title() stricter

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é <saghul@gmail.com>

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)
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }

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;
 }
 

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;

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
 }