From f09179f91aea0ca0b427a5097c0932dc64589fe0 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 7 Jan 2019 20:32:01 -0800 Subject: 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 --- src/plugins/unittest/string_test.c | 11 ++++++++++- src/vppinfra/string.h | 5 +++-- 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); -- cgit 1.2.3-korg