Fix off-by-1 bug in strnfcpy, add test cases

Co-authored-by: Michael Bazzinotti <mbazzinotti@gmail.com>

diff --git a/mutt/string.c b/mutt/string.c
index a694da8fec5e78b1415529f7f5d7db13fa1cafc2..b079aaf88d2625916e3aa4abd6a997a6509995b1 100644 (file)
--- a/mutt/string.c
+++ b/mutt/string.c
@@ -672,15 +672,18 @@ void mutt_str_remove_trailing_ws(char *s)
 
 /**
  * mutt_str_strfcpy - Copy a string into a buffer (guaranteeing NUL-termination)
- * @param dest Buffer for the result
- * @param src  String to copy
- * @param dlen Length of buffer
+ * @param dest  Buffer for the result
+ * @param src   String to copy
+ * @param dsize Destination buffer size
  * @retval len Destination string length
  */
-size_t mutt_str_strfcpy(char *dest, const char *src, size_t dlen)
+size_t mutt_str_strfcpy(char *dest, const char *src, size_t dsize)
 {
+  if (dsize == 0)
+    return 0;
+
   char *dest0 = dest;
-  while ((--dlen > 0) && (*src != '\0'))
+  while ((--dsize > 0) && (*src != '\0'))
     *dest++ = *src++;
 
   *dest = '\0';
@@ -714,17 +717,15 @@ int mutt_str_is_email_wsp(char c)
 
 /**
  * mutt_str_strnfcpy - Copy a limited string into a buffer (guaranteeing NUL-termination)
- * @param dest Buffer for the result
- * @param src  String to copy
- * @param size Maximum number of characters to copy
- * @param dlen Length of buffer
+ * @param dest  Buffer for the result
+ * @param src   String to copy
+ * @param n     Maximum number of characters to copy
+ * @param dsize Destination buffer size
  * @retval len Destination string length
  */
-size_t mutt_str_strnfcpy(char *dest, char *src, size_t size, size_t dlen)
+size_t mutt_str_strnfcpy(char *dest, const char *src, size_t n, size_t dsize)
 {
-  if (dlen > size)
-    dlen = size - 1;
-  return mutt_str_strfcpy(dest, src, dlen);
+  return mutt_str_strfcpy(dest, src, MIN(n + 1, dsize));
 }
 
 /**

diff --git a/mutt/string2.h b/mutt/string2.h
index ba4af69891353775a4a93728ea189863a154d3e4..4f4c4c2ad699e40a0990510d40937f440bf90ae6 100644 (file)
--- a/mutt/string2.h
+++ b/mutt/string2.h
@@ -88,14 +88,14 @@ const char *mutt_str_strchrnul(const char *s, char c);
 int         mutt_str_strcmp(const char *a, const char *b);
 int         mutt_str_strcoll(const char *a, const char *b);
 char *      mutt_str_strdup(const char *s);
-size_t      mutt_str_strfcpy(char *dest, const char *src, size_t dlen);
+size_t      mutt_str_strfcpy(char *dest, const char *src, size_t dsize);
 const char *mutt_str_stristr(const char *haystack, const char *needle);
 size_t      mutt_str_strlen(const char *a);
 char *      mutt_str_strlower(char *s);
 int         mutt_str_strncasecmp(const char *a, const char *b, size_t l);
 char *      mutt_str_strncat(char *d, size_t l, const char *s, size_t sl);
 int         mutt_str_strncmp(const char *a, const char *b, size_t l);
-size_t      mutt_str_strnfcpy(char *dest, char *src, size_t size, size_t dlen);
+size_t      mutt_str_strnfcpy(char *dest, const char *src, size_t n, size_t dsize);
 char *      mutt_str_substr_cpy(char *dest, const char *begin, const char *end, size_t destlen);
 char *      mutt_str_substr_dup(const char *begin, const char *end);
 const char *mutt_str_sysexit(int e);

diff --git a/rfc1524.c b/rfc1524.c
index 260b7a668800610ba117215226bb6980d1babe86..b5277f255982bfeede5175886933b0d4caa1bdd6 100644 (file)
--- a/rfc1524.c
+++ b/rfc1524.c
@@ -558,7 +558,7 @@ int rfc1524_expand_filename(char *nametemplate, char *oldfile, char *newfile, si
       if (lmatch)
         *left = 0;
       else
-        mutt_str_strnfcpy(left, nametemplate, sizeof(left), i);
+        mutt_str_strnfcpy(left, nametemplate, i, sizeof(left));
 
       if (rmatch)
         *right = 0;

diff --git a/test/main.c b/test/main.c
index ad74bb77ec9f43f03a9f67df9b66dce3db8f250d..47d73725ca26d4f1b21cf4bc2e9c3ca07a9d41ce 100644 (file)
--- a/test/main.c
+++ b/test/main.c
@@ -11,7 +11,8 @@
   NEOMUTT_TEST_ITEM(test_md5)                                                  \
   NEOMUTT_TEST_ITEM(test_md5_ctx)                                              \
   NEOMUTT_TEST_ITEM(test_md5_ctx_bytes)                                        \
-  NEOMUTT_TEST_ITEM(test_string_strfcpy)
+  NEOMUTT_TEST_ITEM(test_string_strfcpy)                                       \
+  NEOMUTT_TEST_ITEM(test_string_strnfcpy)
 
 /******************************************************************************
  * You probably don't need to touch what follows.

diff --git a/test/string.c b/test/string.c
index 128e6aea57cd8729db8686ac228f7dfcd08b4dc9..00b7838445f5bb76f16d0febcdc759bc4902e8e9 100644 (file)
--- a/test/string.c
+++ b/test/string.c
@@ -44,3 +44,66 @@ void test_string_strfcpy(void)
     }
   }
 }
+
+void test_string_strnfcpy(void)
+{
+  const char src[] = "One Two Three Four Five";
+  char dst[10];
+  char big[32];
+
+  { /* copy a substring */
+    size_t len = mutt_str_strnfcpy(dst, src, 3, sizeof(dst));
+    if (!TEST_CHECK(len == 3))
+    {
+      TEST_MSG("Expected: %zu", 3);
+      TEST_MSG("Actual  : %zu", len);
+    }
+    if (!TEST_CHECK(strcmp(dst, "One") == 0))
+    {
+      TEST_MSG("Expected: %s", "One");
+      TEST_MSG("Actual  : %s", dst);
+    }
+  }
+
+  { /* try to copy the whole available length */
+    size_t len = mutt_str_strnfcpy(dst, src, sizeof(dst), sizeof(dst));
+    if (!TEST_CHECK(len == sizeof(dst) - 1))
+    {
+      TEST_MSG("Expected: %zu", sizeof(dst) - 1);
+      TEST_MSG("Actual  : %zu", len);
+    }
+    if (!TEST_CHECK(strcmp(dst, "One Two T") == 0))
+    {
+      TEST_MSG("Expected: %s", "One Two T");
+      TEST_MSG("Actual  : %s", dst);
+    }
+  }
+
+  { /* try to copy more than it fits */
+    size_t len = mutt_str_strnfcpy(dst, src, strlen(src), sizeof(dst));
+    if (!TEST_CHECK(len == sizeof(dst) - 1))
+    {
+      TEST_MSG("Expected: %zu", sizeof(dst) - 1);
+      TEST_MSG("Actual  : %zu", len);
+    }
+    if (!TEST_CHECK(strcmp(dst, "One Two T") == 0))
+    {
+      TEST_MSG("Expected: %s", "One Two T");
+      TEST_MSG("Actual  : %s", dst);
+    }
+  }
+
+  { /* try to copy more than available */
+    size_t len = mutt_str_strnfcpy(big, src, sizeof(big), sizeof(big));
+    if (!TEST_CHECK(len == sizeof(src) - 1))
+    {
+      TEST_MSG("Expected: %zu", sizeof(src) - 1);
+      TEST_MSG("Actual  : %zu", len);
+    }
+    if (!TEST_CHECK(strcmp(big, src) == 0))
+    {
+      TEST_MSG("Expected: %s", src);
+      TEST_MSG("Actual  : %s", big);
+    }
+  }
+}