From 7fc60fbf59fbcb952b6d4e65c88eb389c3fa6f74 Mon Sep 17 00:00:00 2001
From: Glenn Smith <couleeapps@gmail.com>
Date: Thu, 8 Mar 2018 00:45:24 -0500
Subject: [PATCH] Better to use strlcat and strlcpy and move them to the cpp
 file. Provided an implementation for platforms that don't support them (macOS
 only currently)

---
 .../source/core/strings/stringFunctions.cpp   | 61 +++++++++++++++++++
 Engine/source/core/strings/stringFunctions.h  | 32 +++++-----
 2 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/Engine/source/core/strings/stringFunctions.cpp b/Engine/source/core/strings/stringFunctions.cpp
index 93db4fd47..668c9d649 100644
--- a/Engine/source/core/strings/stringFunctions.cpp
+++ b/Engine/source/core/strings/stringFunctions.cpp
@@ -381,6 +381,67 @@ char* dStrlwr(char *str)
 #endif
 }
 
+//------------------------------------------------------------------------------
+
+S32 dStrlcat(char *dst, const char *src, dsize_t dstSize)
+{
+   //TODO: Do other platforms support strlcat in their libc
+#ifdef TORQUE_OS_MAC
+   S32 len = strlcat(dst, src, dstSize);
+
+   AssertWarn(len < dstSize, "Buffer too small in call to dStrlcat!");
+
+   return len;
+#else //TORQUE_OS_MAC
+   S32 dstLen = dStrlen(dst);
+   S32 srcLen = dStrlen(src);
+   S32 copyLen = srcLen;
+
+   //Check for buffer overflow and don't allow it. Warn on debug so we can fix it
+   AssertWarn(dstLen + copyLen < dstSize, "Buffer too small in call to dStrlcat!");
+   if (dstLen + copyLen + 1 > dstSize)
+   {
+      copyLen = dstSize - dstLen - 1;
+   }
+
+   //Copy src after dst and null terminate
+   memcpy(dst + dstLen, src, copyLen);
+   dst[dstLen + copyLen] = 0;
+
+   //Return the length of the string we would have generated
+   return dstLen + srcLen;
+#endif //TORQUE_OS_MAC
+}
+
+S32 dStrlcpy(char *dst, const char *src, dsize_t dstSize)
+{
+   //TODO: Do other platforms support strlcpy in their libc
+#ifdef TORQUE_OS_MAC
+   S32 len = strlcpy(dst, src, dstSize);
+
+   AssertWarn(len < dstSize, "Buffer too small in call to dStrlcpy!");
+
+   return len;
+#else //TORQUE_OS_MAC
+   S32 srcLen = dStrlen(src);
+   S32 copyLen = srcLen;
+
+   //Check for buffer overflow and don't allow it. Warn on debug so we can fix it
+   AssertWarn(copyLen < dstSize, "Buffer too small in call to dStrlcpy!");
+   if (srcLen + 1 > dstSize)
+   {
+      copyLen = dstSize - 1;
+   }
+
+   //Copy src and null terminate
+   memcpy(dst, src, copyLen);
+   dst[copyLen] = 0;
+
+   //Return the length of the string we would have generated
+   return srcLen;
+#endif //TORQUE_OS_MAC
+}
+
 //------------------------------------------------------------------------------
 // standard I/O functions
 
diff --git a/Engine/source/core/strings/stringFunctions.h b/Engine/source/core/strings/stringFunctions.h
index c296da871..918273423 100644
--- a/Engine/source/core/strings/stringFunctions.h
+++ b/Engine/source/core/strings/stringFunctions.h
@@ -47,11 +47,14 @@
 
 #endif // defined(TORQUE_OS_WIN)
 
-#define DEBUG_CHECK_STRING_OVERFLOW
-
 //------------------------------------------------------------------------------
 // standard string functions [defined in platformString.cpp]
 
+// Buffer size bounds checking "safe" versions of strcat and strcpy. Ideally you
+// should use these and check if they return >= dstSize and throw an error if so.
+extern S32  dStrlcat(char *dst, const char *src, dsize_t dstSize);
+extern S32  dStrlcpy(char *dst, const char *src, dsize_t dstSize);
+
 #ifdef UNSAFE_STRING_FUNCTIONS
 /// @deprecated Use dStrcat(char *, const char *, dsize_t) instead
 inline char *dStrcat(char *dst, const char *src)
@@ -61,14 +64,15 @@ inline char *dStrcat(char *dst, const char *src)
 }   
 #endif
 
-inline char *dStrcat(char *dst, const char *src, dsize_t len)
+/// Concatenate strings.
+/// @note The third parameter is the size of the destination buffer like strlcat
+///       instead of the number of characters to copy like strncat. This is done
+///       under the assumption that being easier to use will make this safer.
+///       If you want the original behavior use dStrncat.
+inline char *dStrcat(char *dst, const char *src, dsize_t dstSize)
 {
-#ifdef DEBUG_CHECK_STRING_OVERFLOW
-   if (strlen(src) >= len) {
-      AssertWarn(false, "dStrcat out of range");
-   }
-#endif
-   return strncat(dst,src,len - 1); //Safety because strncat copies at most len+1 characters
+   dStrlcat(dst, src, dstSize);
+   return dst;
 }
 
 inline char *dStrncat(char *dst, const char *src, dsize_t len)
@@ -110,14 +114,10 @@ inline char *dStrcpy(char *dst, const char *src)
 }
 #endif
 
-inline char *dStrcpy(char *dst, const char *src, dsize_t len)
+inline char *dStrcpy(char *dst, const char *src, dsize_t dstSize)
 {
-#ifdef DEBUG_CHECK_STRING_OVERFLOW
-   if (strlen(src) >= len) {
-      AssertWarn(false, "dStrcpy out of range");
-   }
-#endif
-   return strncpy(dst,src,len);
+   dStrlcpy(dst, src, dstSize);
+   return dst;
 }   
 
 inline char *dStrncpy(char *dst, const char *src, dsize_t len)