unix: harden string copying, introduce strscpy()

Replace calls to strcpy() and strncpy() with the newly introduced
uv__strscpy() function that is meticulous about zero-terminating
the destination buffer.

PR-URL: https://github.com/libuv/libuv/pull/2065
Refs: https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.html
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

CMakeLists.txt

@@ -15,6 +15,7 @@ set(uv_sources
     src/fs-poll.c
     src/idna.c
     src/inet.c
+    src/strscpy.c
     src/threadpool.c
     src/timer.c
     src/uv-common.c
@@ -116,6 +117,7 @@ set(uv_test_sources
     test/test-socket-buffer-size.c
     test/test-spawn.c
     test/test-stdio-over-pipes.c
+    test/test-strscpy.c
     test/test-tcp-alloc-cb-fail.c
     test/test-tcp-bind-error.c
     test/test-tcp-bind6-error.c

Makefile.am

@@ -32,6 +32,8 @@ libuv_la_SOURCES = src/fs-poll.c \
                    src/idna.c \
                    src/inet.c \
                    src/queue.h \
+                   src/strscpy.c \
+                   src/strscpy.h \
                    src/threadpool.c \
                    src/timer.c \
                    src/uv-data-getter-setters.c \
@@ -241,6 +243,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \
                          test/test-socket-buffer-size.c \
                          test/test-spawn.c \
                          test/test-stdio-over-pipes.c \
+                         test/test-strscpy.c \
                          test/test-tcp-alloc-cb-fail.c \
                          test/test-tcp-bind-error.c \
                          test/test-tcp-bind6-error.c \

include/uv/win.h

@@ -25,6 +25,7 @@
 
 #if !defined(_SSIZE_T_) && !defined(_SSIZE_T_DEFINED)
 typedef intptr_t ssize_t;
+# define SSIZE_MAX INTPTR_MAX
 # define _SSIZE_T_
 # define _SSIZE_T_DEFINED
 #endif

src/inet.c

@@ -59,8 +59,7 @@ static int inet_ntop4(const unsigned char *src, char *dst, size_t size) {
   if (l <= 0 || (size_t) l >= size) {
     return UV_ENOSPC;
   }
-  strncpy(dst, tmp, size);
-  dst[size - 1] = '\0';
+  uv__strscpy(dst, tmp, size);
   return 0;
 }
 
@@ -142,14 +141,8 @@ static int inet_ntop6(const unsigned char *src, char *dst, size_t size) {
   if (best.base != -1 && (best.base + best.len) == ARRAY_SIZE(words))
     *tp++ = ':';
   *tp++ = '\0';
-
-  /*
-   * Check for overflow, copy, and we're done.
-   */
-  if ((size_t)(tp - tmp) > size) {
+  if (UV_E2BIG == uv__strscpy(dst, tmp, size))
     return UV_ENOSPC;
-  }
-  strcpy(dst, tmp);
   return 0;
 }
 

src/strscpy.c

@@ -0,0 +1,17 @@
+#include "strscpy.h"
+#include <limits.h>  /* SSIZE_MAX */
+
+ssize_t uv__strscpy(char* d, const char* s, size_t n) {
+  size_t i;
+
+  for (i = 0; i < n; i++)
+    if ('\0' == (d[i] = s[i]))
+      return i > SSIZE_MAX ? UV_E2BIG : (ssize_t) i;
+
+  if (i == 0)
+    return 0;
+
+  d[--i] = '\0';
+
+  return UV_E2BIG;
+}

src/strscpy.h

@@ -0,0 +1,18 @@
+#ifndef UV_STRSCPY_H_
+#define UV_STRSCPY_H_
+
+/* Include uv.h for its definitions of size_t and ssize_t.
+ * size_t can be obtained directly from <stddef.h> but ssize_t requires
+ * some hoop jumping on Windows that I didn't want to duplicate here.
+ */
+#include "uv.h"
+
+/* Copies up to |n-1| bytes from |d| to |s| and always zero-terminates
+ * the result, except when |n==0|. Returns the number of bytes copied
+ * or UV_E2BIG if |d| is too small.
+ *
+ * See https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.html
+ */
+ssize_t uv__strscpy(char* d, const char* s, size_t n);
+
+#endif  /* UV_STRSCPY_H_ */

src/unix/aix.c

@@ -366,7 +366,8 @@ static char *uv__rawname(char *cp) {
     return 0;
 
   *dp = 0;
-  strcpy(rawbuf, cp);
+  /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+  uv__strscpy(rawbuf, cp, sizeof(rawbuf));
   *dp = '/';
   strcat(rawbuf, "/r");
   strcat(rawbuf, dp+1);
@@ -453,7 +454,8 @@ static int uv__makedir_p(const char *dir) {
   size_t len;
   int err;
 
-  snprintf(tmp, sizeof(tmp),"%s",dir);
+  /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+  uv__strscpy(tmp, dir, sizeof(tmp));
   len = strlen(tmp);
   if (tmp[len - 1] == '/')
     tmp[len - 1] = 0;
@@ -702,9 +704,9 @@ static void uv__ahafs_event(uv_loop_t* loop, uv__io_t* event_watch, unsigned int
     else
       p++;
   }
-  strncpy(fname, p, sizeof(fname) - 1);
-  /* Just in case */
-  fname[sizeof(fname) - 1] = '\0';
+
+  /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+  uv__strscpy(fname, p, sizeof(fname));
 
   handle->cb(handle, fname, events, 0);
 }
@@ -735,7 +737,8 @@ int uv_fs_event_start(uv_fs_event_t* handle,
   /* Figure out whether filename is absolute or not */
   if (filename[0] == '/') {
     /* We have absolute pathname */
-    snprintf(absolute_path, sizeof(absolute_path), "%s", filename);
+    /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+    uv__strscpy(absolute_path, filename, sizeof(absolute_path));
   } else {
     /* We have a relative pathname, compose the absolute pathname */
     snprintf(cwd, sizeof(cwd), "/proc/%lu/cwd", (unsigned long) getpid());
@@ -986,7 +989,8 @@ int uv_cpu_info(uv_cpu_info_t** cpu_infos, int* count) {
     return UV_ENOMEM;
   }
 
-  strcpy(cpu_id.name, FIRST_CPU);
+  /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+  uv__strscpy(cpu_id.name, FIRST_CPU, sizeof(cpu_id.name));
   result = perfstat_cpu(&cpu_id, ps_cpus, sizeof(perfstat_cpu_t), ncpus);
   if (result == -1) {
     uv__free(ps_cpus);

src/unix/darwin-proctitle.c

@@ -192,8 +192,7 @@ void uv__set_process_title(const char* title) {
 
   if (dynamic_pthread_setname_np != NULL) {
     char namebuf[64];  /* MAXTHREADNAMESIZE */
-    strncpy(namebuf, title, sizeof(namebuf) - 1);
-    namebuf[sizeof(namebuf) - 1] = '\0';
+    uv__strscpy(namebuf, title, sizeof(namebuf));
     dynamic_pthread_setname_np(namebuf);
   }
 }

src/unix/netbsd.c

@@ -87,7 +87,8 @@ int uv_exepath(char* buffer, size_t* size) {
   /* Copy string from the intermediate buffer to outer one with appropriate
    * length.
    */
-  strlcpy(buffer, int_buf, *size);
+  /* TODO(bnoordhuis) Check uv__strscpy() return value. */
+  uv__strscpy(buffer, int_buf, *size);
 
   /* Set new size. */
   *size = strlen(buffer);

src/unix/os390.c

@@ -237,7 +237,7 @@ static int getexe(const int pid, char* buf, size_t len) {
     return -1;
   }
 
-  strncpy(buf, Output_path->path, len);
+  uv__strscpy(buf, Output_path->path, len);
 
   return 0;
 }

src/unix/pipe.c

@@ -66,8 +66,7 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) {
   sockfd = err;
 
   memset(&saddr, 0, sizeof saddr);
-  strncpy(saddr.sun_path, pipe_fname, sizeof(saddr.sun_path) - 1);
-  saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0';
+  uv__strscpy(saddr.sun_path, pipe_fname, sizeof(saddr.sun_path));
   saddr.sun_family = AF_UNIX;
 
   if (bind(sockfd, (struct sockaddr*)&saddr, sizeof saddr)) {
@@ -186,8 +185,7 @@ void uv_pipe_connect(uv_connect_t* req,
   }
 
   memset(&saddr, 0, sizeof saddr);
-  strncpy(saddr.sun_path, name, sizeof(saddr.sun_path) - 1);
-  saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0';
+  uv__strscpy(saddr.sun_path, name, sizeof(saddr.sun_path));
   saddr.sun_family = AF_UNIX;
 
   do {

src/uv-common.c

@@ -162,7 +162,7 @@ static const char* uv__unknown_err_code(int err) {
 
 #define UV_ERR_NAME_GEN_R(name, _) \
 case UV_## name: \
-  snprintf(buf, buflen, "%s", #name); break;
+  uv__strscpy(buf, #name, buflen); break;
 char* uv_err_name_r(int err, char* buf, size_t buflen) {
   switch (err) {
     UV_ERRNO_MAP(UV_ERR_NAME_GEN_R)

src/uv-common.h

@@ -40,6 +40,7 @@
 #include "uv.h"
 #include "uv/tree.h"
 #include "queue.h"
+#include "strscpy.h"
 
 #if EDOM > 0
 # define UV__ERR(x) (-(x))

test/test-list.h

@@ -352,6 +352,7 @@ TEST_DECLARE   (fs_exclusive_sharing_mode)
 TEST_DECLARE   (fs_open_readonly_acl)
 TEST_DECLARE   (fs_fchmod_archive_readonly)
 #endif
+TEST_DECLARE   (strscpy)
 TEST_DECLARE   (threadpool_queue_work_simple)
 TEST_DECLARE   (threadpool_queue_work_einval)
 TEST_DECLARE   (threadpool_multiple_event_loops)
@@ -909,6 +910,7 @@ TASK_LIST_START
 #endif
   TEST_ENTRY  (get_osfhandle_valid_handle)
   TEST_ENTRY  (open_osfhandle_valid_handle)
+  TEST_ENTRY  (strscpy)
   TEST_ENTRY  (threadpool_queue_work_simple)
   TEST_ENTRY  (threadpool_queue_work_einval)
   TEST_ENTRY  (threadpool_multiple_event_loops)

test/test-strscpy.c

@@ -0,0 +1,53 @@
+/* Copyright libuv project contributors. All rights reserved.
+*
+* Permission is hereby granted, free of charge, to any person obtaining a copy
+* of this software and associated documentation files (the "Software"), to
+* deal in the Software without restriction, including without limitation the
+* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+* sell copies of the Software, and to permit persons to whom the Software is
+* furnished to do so, subject to the following conditions:
+*
+* The above copyright notice and this permission notice shall be included in
+* all copies or substantial portions of the Software.
+*
+* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+* IN THE SOFTWARE.
+*/
+
+#include "uv.h"
+#include "task.h"
+#include <string.h>
+
+#include "../src/strscpy.h"
+#include "../src/strscpy.c"
+
+TEST_IMPL(strscpy) {
+  char d[4];
+
+  ASSERT(0 == uv__strscpy(d, "", 0));
+  ASSERT(0 == uv__strscpy(d, "x", 0));
+
+  memset(d, 0, sizeof(d));
+  ASSERT(1 == uv__strscpy(d, "x", sizeof(d)));
+  ASSERT(0 == memcmp(d, "x\0\0", sizeof(d)));
+
+  memset(d, 0, sizeof(d));
+  ASSERT(2 == uv__strscpy(d, "xy", sizeof(d)));
+  ASSERT(0 == memcmp(d, "xy\0", sizeof(d)));
+
+  ASSERT(3 == uv__strscpy(d, "xyz", sizeof(d)));
+  ASSERT(0 == memcmp(d, "xyz", sizeof(d)));
+
+  ASSERT(UV_E2BIG == uv__strscpy(d, "xyzz", sizeof(d)));
+  ASSERT(0 == memcmp(d, "xyz", sizeof(d)));
+
+  ASSERT(UV_E2BIG == uv__strscpy(d, "xyzzy", sizeof(d)));
+  ASSERT(0 == memcmp(d, "xyz", sizeof(d)));
+
+  return 0;
+}

test/test.gyp

@@ -34,6 +34,7 @@
         'test-fs.c',
         'test-fs-copyfile.c',
         'test-fs-event.c',
+        'test-fs-poll.c',
         'test-getters-setters.c',
         'test-get-currentexe.c',
         'test-get-memory.c',
@@ -96,7 +97,7 @@
         'test-signal-multiple-loops.c',
         'test-socket-buffer-size.c',
         'test-spawn.c',
-        'test-fs-poll.c',
+        'test-strscpy.c',
         'test-stdio-over-pipes.c',
         'test-tcp-alloc-cb-fail.c',
         'test-tcp-bind-error.c',

uv.gyp

@@ -74,6 +74,8 @@
         'src/idna.h',
         'src/inet.c',
         'src/queue.h',
+        'src/strscpy.c',
+        'src/strscpy.h',
         'src/threadpool.c',
         'src/timer.c',
         'src/uv-data-getter-setters.c',