strncpy_s_inline copies more bytes than necessary

Given n equals to the maximum number of bytes to copy from src in the API,
or the rough estimate strlen of src, strncpy_s_inline should not copy more
than the number of bytes, computed by strlen(src), to dst if n is greater than
strlen(src). The number of bytes to copy is computed by strnlen(src,n), not n.

Change-Id: I088b46125d9776962750e121f1fbf441952efc2b
Signed-off-by: Steven <sluong@cisco.com>

2 files changed, 13 insertions, 3 deletions

diff --git a/src/plugins/unittest/string_test.c b/src/plugins/unittest/string_test.c
index 0d41bb27918..41b4c61dff4 100644
--- a/src/plugins/unittest/string_test.c
+++ b/src/plugins/unittest/string_test.c
@@ -628,7 +628,7 @@ test_strncpy_s (vlib_main_t * vm, unformat_input_t * input)
 {
   char src[] = "Those who dare to fail miserably can achieve greatly.";
   char dst[100], old_dst[100];
-  int indicator;
+  int indicator, i;
   size_t s1size = sizeof (dst);	// including null
   errno_t err;
 
@@ -658,6 +658,10 @@ test_strncpy_s (vlib_main_t * vm, unformat_input_t * input)
     return -1;
 
   /* n > string len of src */
+  err = clib_memset (dst, 1, sizeof (dst));
+  if (err != EOK)
+    return -1;
+
   err = strncpy_s (dst, s1size, src, clib_strnlen (src, sizeof (src)) + 10);
   if (err != EOK)
     return -1;
@@ -667,6 +671,11 @@ test_strncpy_s (vlib_main_t * vm, unformat_input_t * input)
   if (indicator != 0)
     return -1;
 
+  /* Make sure bytes after strlen(dst) is untouched */
+  for (i = 1 + clib_strnlen (dst, sizeof (dst)); i < sizeof (dst); i++)
+    if (dst[i] != 1)
+      return -1;
+
   /* truncation, n >= dmax */
   err = strncpy_s (dst, clib_strnlen (src, sizeof (src)), src,
 		   clib_strnlen (src, sizeof (src)));
diff --git a/src/vppinfra/string.h b/src/vppinfra/string.h
index d5686704c22..42f7890f3d0 100644
--- a/src/vppinfra/string.h
+++ b/src/vppinfra/string.h
@@ -103,7 +103,7 @@ void clib_memswap (void *_a, void *_b, uword bytes);
  * In order to provide smooth mapping from unsafe string API to the clib string
  * macro, we often have to improvise s1max and s2max due to the additional
  * arguments are required for implementing the safe API. This macro is used
- * to provide the s1max/s2max. It is not perfect becuase the actual
+ * to provide the s1max/s2max. It is not perfect because the actual
  * s1max/s2max may be greater than 4k and the mapping from the unsafe API to
  * the macro would cause a regression. However, it is not terribly likely.
  * So I bet against the odds.
@@ -1025,7 +1025,8 @@ strncpy_s_inline (char *__restrict__ dest, rsize_t dmax,
 	}
     }
   else
-    m = n;
+    /* cap the copy to strlen(src) in case n > strlen(src) */
+    m = clib_strnlen (src, n);
 
   /* Check for src/dst overlap, which is not allowed */
   low = (uword) (src < dest ? src : dest);