From 948f23417010309a5557d46195eae258f6105025 Mon Sep 17 00:00:00 2001
From: Sebastian Witt <sebastian.witt@siemens.com>
Date: Tue, 4 Jun 2024 13:38:26 +0200
Subject: CryptoPkg: Fix BaseCryptLib CrtWrapper strncpy and strcat

Following https://bugzilla.tianocore.org/show_bug.cgi?id=2817 this
bug could also apply to strncpy and strcat.

For strncpy use count+1 if smaller than MAX_STRING_SIZE. This still
restricts the destination size to MAX_STRING_SIZE as before but allows
a strncpy when the source is close after destination without triggering
the InternalSafeStringNoAsciiStrOverlap check in AsciiStrnCpyS.

For strcat use the destination string length + the size of the source
string including the terminator as destination size if smaller than
MAX_STRING_SIZE.

Also move both functions to CrtWrapper.c as they do not return the
correct return value. AsciiStrnCpyS and AsciiStrCatS return
RETURN_VALUE instead of a char * to the destination buffer.

Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com>
---
 .../Library/BaseCryptLib/SysCall/CrtWrapper.c      | 41 +++++++++++++++++++++-
 CryptoPkg/Library/Include/CrtLibSupport.h          | 37 ++++++++++++-------
 2 files changed, 64 insertions(+), 14 deletions(-)

(limited to 'CryptoPkg')

diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
index 880ed14..b114cc0 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
@@ -271,7 +271,46 @@ strcpy (
   const char  *strSource
   )
 {
-  AsciiStrCpyS (strDest, AsciiStrnSizeS (strSource, MAX_STRING_SIZE), strSource);
+  AsciiStrCpyS (strDest, AsciiStrnSizeS (strSource, MAX_STRING_SIZE - 1), strSource);
+  return strDest;
+}
+
+char *
+strncpy (
+  char        *strDest,
+  const char  *strSource,
+  size_t      count
+  )
+{
+  UINTN  DestMax = MAX_STRING_SIZE;
+
+  if (count < MAX_STRING_SIZE) {
+    DestMax = count + 1;
+  } else {
+    count = MAX_STRING_SIZE-1;
+  }
+
+  AsciiStrnCpyS (strDest, DestMax, strSource, (UINTN)count);
+
+  return strDest;
+}
+
+char *
+strcat (
+  char        *strDest,
+  const char  *strSource
+  )
+{
+  UINTN  DestMax;
+
+  DestMax = AsciiStrnLenS (strDest, MAX_STRING_SIZE) + AsciiStrnSizeS (strSource, MAX_STRING_SIZE);
+
+  if (DestMax > MAX_STRING_SIZE) {
+    DestMax = MAX_STRING_SIZE;
+  }
+
+  AsciiStrCatS (strDest, DestMax, strSource);
+
   return strDest;
 }
 
diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
index f36fe08..afc0095 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -403,22 +403,33 @@ strcpy (
   const char  *strSource
   );
 
+char *
+strncpy (
+  char        *strDest,
+  const char  *strSource,
+  size_t      count
+  );
+
+char *
+strcat (
+  char        *strDest,
+  const char  *strSource
+  );
+
 //
 // Macros that directly map functions to BaseLib, BaseMemoryLib, and DebugLib functions
 //
-#define memcpy(dest, source, count)         CopyMem(dest,source,(UINTN)(count))
-#define memset(dest, ch, count)             SetMem(dest,(UINTN)(count),(UINT8)(ch))
-#define memchr(buf, ch, count)              ScanMem8(buf,(UINTN)(count),(UINT8)ch)
-#define memcmp(buf1, buf2, count)           (int)(CompareMem(buf1,buf2,(UINTN)(count)))
-#define memmove(dest, source, count)        CopyMem(dest,source,(UINTN)(count))
-#define strlen(str)                         (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
-#define strncpy(strDest, strSource, count)  AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
-#define strcat(strDest, strSource)          AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
-#define strncmp(string1, string2, count)    (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
-#define strcasecmp(str1, str2)              (int)AsciiStriCmp(str1,str2)
-#define strstr(s1, s2)                      AsciiStrStr(s1,s2)
-#define sprintf(buf, ...)                   AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
-#define localtime(timer)                    NULL
+#define memcpy(dest, source, count)       CopyMem(dest,source,(UINTN)(count))
+#define memset(dest, ch, count)           SetMem(dest,(UINTN)(count),(UINT8)(ch))
+#define memchr(buf, ch, count)            ScanMem8(buf,(UINTN)(count),(UINT8)ch)
+#define memcmp(buf1, buf2, count)         (int)(CompareMem(buf1,buf2,(UINTN)(count)))
+#define memmove(dest, source, count)      CopyMem(dest,source,(UINTN)(count))
+#define strlen(str)                       (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
+#define strncmp(string1, string2, count)  (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
+#define strcasecmp(str1, str2)            (int)AsciiStriCmp(str1,str2)
+#define strstr(s1, s2)                    AsciiStrStr(s1,s2)
+#define sprintf(buf, ...)                 AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
+#define localtime(timer)                  NULL
 #define assert(expression)
 #define offsetof(type, member)  OFFSET_OF(type,member)
 #define atoi(nptr)              AsciiStrDecimalToUintn(nptr)
-- 
cgit v1.1