[Patch v9 0/2] Disable safe string constraint assertions

[Patch v9 0/2] Disable safe string constraint assertions

[Michael D Kinney]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054

V9 Update unit test comment and use zero buffers to verify more behavior.
V8 Add DEBUG_VERBOSE message and unit test.
V7 addressed review comments (only documentation changes).

Current implementation of SafeString does not let one parse untrusted
data with its interfaces as they ASSERT on failing runtime checks and
require one to effectively reimplement the function on the caller side.

All the former proposals made it clear that the attempts to introduce
a solution preserving this behavior require a lot of changes
throughout the codebase including platform code (e.g. introducing
constraint assertions and updating all DebugLib implementations)
or require all sorts of hacks.

Since the original code has roughly no benefit except in some very
special cases and the effort required to preserve it is very high,
I propose to remove it as agreed on with several other parties.

Please note, that this patch does not change the behaviour of the
functions in RELEASE builds. I.e. they will still check for NULL
and return RETURN_INVALID_PARAMETER. In the future we may (or may
not) want them to simply ASSERT in this case. Regardless this should
be done in a separate BZ entry and a separate commit. For this reason
I ask everyone not to touch this subject.

Due to the amount of discussion this has already undergone after
almost a year I kindly request everyone to reduce the communication
to the minimum and abstain from proposing another approach.

Requesting to merge this into edk2-stable202005.

Cc: Andrew Fish afish@apple.com
Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
Cc: Bret Barkelew bret.barkelew@microsoft.com
Cc: Brian J. Johnson brian.johnson@hpe.com
Cc: Chasel Chiu chasel.chiu@intel.com
Cc: Jordan Justen jordan.l.justen@intel.com
Cc: Laszlo Ersek lersek@redhat.com
Cc: Leif Lindholm leif@nuviainc.com
Cc: Liming Gao liming.gao@intel.com
Cc: Marvin Häuser mhaeuser@outlook.de
Cc: Michael D Kinney michael.d.kinney@intel.com
Cc: Vincent Zimmer vincent.zimmer@intel.com
Cc: Zhichao Gao zhichao.gao@intel.com
Cc: Jiewen Yao jiewen.yao@intel.com
Signed-off-by: Vitaly Cheptsov vit9696@protonmail.com

Michael D Kinney (1):
  MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

Vitaly Cheptsov (1):
  MdePkg: Fix SafeString performing assertions on runtime checks

 MdePkg/Include/Library/BaseLib.h              | 111 -----------------
 MdePkg/Library/BaseLib/SafeString.c           | 115 +-----------------
 .../UnitTest/Library/BaseLib/Base64UnitTest.c | 107 ++++++++++++++++
 3 files changed, 110 insertions(+), 223 deletions(-)

-- 
2.21.0.windows.1

[Patch v9 1/2] MdePkg: Fix SafeString performing assertions on runtime checks

[Michael D Kinney]

From: Vitaly Cheptsov <vit9696@protonmail.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054

Runtime checks returned via status return code should not work as
assertions to permit parsing not trusted data with SafeString
interfaces.  Replace ASSERT() with a DEBUG_VERBOSE message.

Cc: Andrew Fish <afish@apple.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Bret Barkelew <bret.barkelew@microsoft.com>
Cc: Brian J. Johnson <brian.johnson@hpe.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Marvin H?user <mhaeuser@outlook.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Vincent Zimmer <vincent.zimmer@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
---
 MdePkg/Include/Library/BaseLib.h    | 111 ---------------------------
 MdePkg/Library/BaseLib/SafeString.c | 115 +---------------------------
 2 files changed, 3 insertions(+), 223 deletions(-)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index b0bbe8cef8..8e7b87cbda 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -216,7 +216,6 @@ StrnSizeS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -252,7 +251,6 @@ StrCpyS (
 
   If Length > 0 and Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Length > 0 and Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -290,7 +288,6 @@ StrnCpyS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -330,7 +327,6 @@ StrCatS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -377,12 +373,7 @@ StrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -433,12 +424,7 @@ StrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -494,12 +480,7 @@ StrDecimalToUint64S (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -555,12 +536,7 @@ StrHexToUintnS (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -649,8 +625,6 @@ AsciiStrnSizeS (
 
   This function is similar as strcpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -683,8 +657,6 @@ AsciiStrCpyS (
 
   This function is similar as strncpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -719,8 +691,6 @@ AsciiStrnCpyS (
 
   This function is similar as strcat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -757,8 +727,6 @@ AsciiStrCatS (
 
   This function is similar as strncat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -804,12 +772,6 @@ AsciiStrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINTN, then
@@ -859,12 +821,6 @@ AsciiStrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINT64, then
@@ -918,12 +874,6 @@ AsciiStrDecimalToUint64S (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINTN, then
@@ -977,12 +927,6 @@ AsciiStrHexToUintnS (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINT64, then
@@ -1533,16 +1477,8 @@ StrHexToUint64 (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -1594,16 +1530,8 @@ StrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -1667,8 +1595,6 @@ StrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
   @param  String                   Pointer to a Null-terminated Unicode string.
@@ -1703,17 +1629,6 @@ StrToGuid (
 
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If String is NULL, then ASSERT().
-
-  If Buffer is NULL, then ASSERT().
-
-  If Length is not multiple of 2, then ASSERT().
-
-  If PcdMaximumUnicodeStringLength is not zero and Length is greater than
-  PcdMaximumUnicodeStringLength, then ASSERT().
-
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
-
   @param  String                   Pointer to a Null-terminated Unicode string.
   @param  Length                   The number of Unicode characters to decode.
   @param  Buffer                   Pointer to the converted bytes array.
@@ -1804,7 +1719,6 @@ UnicodeStrToAsciiStr (
   the upper 8 bits, then ASSERT().
 
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -1851,7 +1765,6 @@ UnicodeStrToAsciiStrS (
   If any Unicode characters in Source contain non-zero value in the upper 8
   bits, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2415,10 +2328,6 @@ AsciiStrHexToUint64 (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -2470,10 +2379,6 @@ AsciiStrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -2535,9 +2440,6 @@ AsciiStrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
-
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Guid                     Pointer to the converted GUID.
 
@@ -2568,17 +2470,6 @@ AsciiStrToGuid (
   decoding stops after Length of characters and outputs Buffer containing
   (Length / 2) bytes.
 
-  If String is NULL, then ASSERT().
-
-  If Buffer is NULL, then ASSERT().
-
-  If Length is not multiple of 2, then ASSERT().
-
-  If PcdMaximumAsciiStringLength is not zero and Length is greater than
-  PcdMaximumAsciiStringLength, then ASSERT().
-
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
-
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Length                   The number of ASCII characters to decode.
   @param  Buffer                   Pointer to the converted bytes array.
@@ -2659,7 +2550,6 @@ AsciiStrToUnicodeStr (
   equal or greater than ((AsciiStrLen (Source) + 1) * sizeof (CHAR16)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2705,7 +2595,6 @@ AsciiStrToUnicodeStrS (
   ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then Destination and DestinationLength are
   unmodified.
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
index 7dc03d2caa..3bb23ca1a1 100644
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -14,8 +14,10 @@
 
 #define SAFE_STRING_CONSTRAINT_CHECK(Expression, Status)  \
   do { \
-    ASSERT (Expression); \
     if (!(Expression)) { \
+      DEBUG ((DEBUG_VERBOSE, \
+        "%a(%d) %a: SAFE_STRING_CONSTRAINT_CHECK(%a) failed.  Return %r\n", \
+        __FILE__, __LINE__, __FUNCTION__, #Expression, Status)); \
       return Status; \
     } \
   } while (FALSE)
@@ -197,7 +199,6 @@ StrnSizeS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -279,7 +280,6 @@ StrCpyS (
 
   If Length > 0 and Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Length > 0 and Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -372,7 +372,6 @@ StrnCpyS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -473,7 +472,6 @@ StrCatS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -590,12 +588,7 @@ StrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -705,12 +698,7 @@ StrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -825,12 +813,7 @@ StrDecimalToUint64S (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -956,12 +939,7 @@ StrHexToUintnS (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -1091,16 +1069,8 @@ StrHexToUint64S (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -1317,16 +1287,8 @@ StrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If PcdMaximumUnicodeStringLength is not zero, and String contains more than
-  PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -1482,8 +1444,6 @@ StrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
   @param  String                   Pointer to a Null-terminated Unicode string.
@@ -1589,17 +1549,6 @@ StrToGuid (
 
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If String is NULL, then ASSERT().
-
-  If Buffer is NULL, then ASSERT().
-
-  If Length is not multiple of 2, then ASSERT().
-
-  If PcdMaximumUnicodeStringLength is not zero and Length is greater than
-  PcdMaximumUnicodeStringLength, then ASSERT().
-
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
-
   @param  String                   Pointer to a Null-terminated Unicode string.
   @param  Length                   The number of Unicode characters to decode.
   @param  Buffer                   Pointer to the converted bytes array.
@@ -1779,8 +1728,6 @@ AsciiStrnSizeS (
 
   This function is similar as strcpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -1856,8 +1803,6 @@ AsciiStrCpyS (
 
   This function is similar as strncpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -1944,8 +1889,6 @@ AsciiStrnCpyS (
 
   This function is similar as strcat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -2040,8 +1983,6 @@ AsciiStrCatS (
 
   This function is similar as strncat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
-
   If an error is returned, then the Destination is unmodified.
 
   @param  Destination              A pointer to a Null-terminated Ascii string.
@@ -2154,12 +2095,6 @@ AsciiStrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINTN, then
@@ -2266,12 +2201,6 @@ AsciiStrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINT64, then
@@ -2382,12 +2311,6 @@ AsciiStrDecimalToUint64S (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINTN, then
@@ -2509,12 +2432,6 @@ AsciiStrHexToUintnS (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
-  If PcdMaximumAsciiStringLength is not zero, and String contains more than
-  PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
-
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
   If the number represented by String exceeds the range defined by UINT64, then
@@ -2635,7 +2552,6 @@ AsciiStrHexToUint64S (
   the upper 8 bits, then ASSERT().
 
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2735,7 +2651,6 @@ UnicodeStrToAsciiStrS (
   If any Unicode characters in Source contain non-zero value in the upper 8
   bits, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then Destination and DestinationLength are
   unmodified.
@@ -2855,7 +2770,6 @@ UnicodeStrnToAsciiStrS (
   equal or greater than ((AsciiStrLen (Source) + 1) * sizeof (CHAR16)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2948,7 +2862,6 @@ AsciiStrToUnicodeStrS (
   ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
 
   If an error is returned, then Destination and DestinationLength are
   unmodified.
@@ -3072,10 +2985,6 @@ AsciiStrnToUnicodeStrS (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -3291,10 +3200,6 @@ AsciiStrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
-
-  If Address is NULL, then ASSERT().
-
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
   by EndPointer.
@@ -3448,9 +3353,6 @@ AsciiStrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
-
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Guid                     Pointer to the converted GUID.
 
@@ -3550,17 +3452,6 @@ AsciiStrToGuid (
   decoding stops after Length of characters and outputs Buffer containing
   (Length / 2) bytes.
 
-  If String is NULL, then ASSERT().
-
-  If Buffer is NULL, then ASSERT().
-
-  If Length is not multiple of 2, then ASSERT().
-
-  If PcdMaximumAsciiStringLength is not zero and Length is greater than
-  PcdMaximumAsciiStringLength, then ASSERT().
-
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
-
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Length                   The number of ASCII characters to decode.
   @param  Buffer                   Pointer to the converted bytes array.
-- 
2.21.0.windows.1

[Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

[Michael D Kinney]

Use the safe string function StrCpyS() in BaseLib to test the
SAFE_STRING_CONSTRAINT_CHECK() macro.

Cc: Andrew Fish <afish@apple.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Bret Barkelew <bret.barkelew@microsoft.com>
Cc: Brian J. Johnson <brian.johnson@hpe.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Marvin H?user <mhaeuser@outlook.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Vincent Zimmer <vincent.zimmer@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 .../UnitTest/Library/BaseLib/Base64UnitTest.c | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
index 8952f9da6c..2c4266491c 100644
--- a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
+++ b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
@@ -290,6 +290,99 @@ RfcDecodeTest(
   return UNIT_TEST_PASSED;
 }
 
+#define SOURCE_STRING  L"Hello"
+
+STATIC
+UNIT_TEST_STATUS
+EFIAPI
+SafeStringContraintCheckTest (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  RETURN_STATUS  Status;
+  CHAR16         Destination[20];
+  CHAR16         AllZero[20];
+
+  //
+  // Zero buffer used to verify Destination is not modified
+  //
+  ZeroMem (AllZero, sizeof (AllZero));
+
+  //
+  // Positive test case copy source unicode string to destination
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), SOURCE_STRING);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING, sizeof (SOURCE_STRING));
+
+  //
+  // Positive test case with DestMax the same as Source size
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, sizeof (SOURCE_STRING) / sizeof (CHAR16), SOURCE_STRING);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING, sizeof (SOURCE_STRING));
+
+  //
+  // Negative test case with Destination NULL
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (NULL, sizeof (Destination) / sizeof (CHAR16), SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with Source NULL
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), NULL);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with DestMax too big
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, MAX_UINTN, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with DestMax 0
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, 0, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with DestMax smaller than Source size
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, 1, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with DestMax smaller than Source size by one character
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, sizeof (SOURCE_STRING) / sizeof (CHAR16) - 1, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  //
+  // Negative test case with overlapping Destination and Source
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), Destination);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_ACCESS_DENIED);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
+
+  return UNIT_TEST_PASSED;
+}
+
 /**
   Initialze the unit test framework, suite, and unit tests for the
   Base64 conversion APIs of BaseLib and run the unit tests.
@@ -309,6 +402,7 @@ UnitTestingEntry (
   UNIT_TEST_FRAMEWORK_HANDLE  Fw;
   UNIT_TEST_SUITE_HANDLE      b64EncodeTests;
   UNIT_TEST_SUITE_HANDLE      b64DecodeTests;
+  UNIT_TEST_SUITE_HANDLE      SafeStringTests;
 
   Fw = NULL;
 
@@ -367,6 +461,19 @@ UnitTestingEntry (
   AddTestCase (b64DecodeTests, "Incorrectly placed padding character", "Error4", RfcDecodeTest, NULL, CleanUpB64TestContext, &mBasicDecodeError4);
   AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", RfcDecodeTest, NULL, CleanUpB64TestContext, &mBasicDecodeError5);
 
+  //
+  // Populate the safe string Unit Test Suite.
+  //
+  Status = CreateUnitTestSuite (&SafeStringTests, Fw, "Safe String", "BaseLib.SafeString", NULL, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for SafeStringTests\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto EXIT;
+  }
+
+  // --------------Suite-----------Description--------------Class Name----------Function--------Pre---Post-------------------Context-----------
+  AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", "SafeStringContraintCheckTest", SafeStringContraintCheckTest, NULL, NULL, NULL);
+
   //
   // Execute the tests.
   //
-- 
2.21.0.windows.1

Re: [edk2-devel] [Patch v9 0/2] Disable safe string constraint assertions

[Michael D Kinney]

PR for V9 with all checks passing:

https://github.com/tianocore/edk2/pull/636

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Michael D Kinney
> Sent: Wednesday, May 20, 2020 1:10 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch v9 0/2] Disable safe
> string constraint assertions
> 
> REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2054
> 
> V9 Update unit test comment and use zero buffers to
> verify more behavior.
> V8 Add DEBUG_VERBOSE message and unit test.
> V7 addressed review comments (only documentation
> changes).
> 
> Current implementation of SafeString does not let one
> parse untrusted
> data with its interfaces as they ASSERT on failing
> runtime checks and
> require one to effectively reimplement the function on
> the caller side.
> 
> All the former proposals made it clear that the
> attempts to introduce
> a solution preserving this behavior require a lot of
> changes
> throughout the codebase including platform code (e.g.
> introducing
> constraint assertions and updating all DebugLib
> implementations)
> or require all sorts of hacks.
> 
> Since the original code has roughly no benefit except
> in some very
> special cases and the effort required to preserve it is
> very high,
> I propose to remove it as agreed on with several other
> parties.
> 
> Please note, that this patch does not change the
> behaviour of the
> functions in RELEASE builds. I.e. they will still check
> for NULL
> and return RETURN_INVALID_PARAMETER. In the future we
> may (or may
> not) want them to simply ASSERT in this case.
> Regardless this should
> be done in a separate BZ entry and a separate commit.
> For this reason
> I ask everyone not to touch this subject.
> 
> Due to the amount of discussion this has already
> undergone after
> almost a year I kindly request everyone to reduce the
> communication
> to the minimum and abstain from proposing another
> approach.
> 
> Requesting to merge this into edk2-stable202005.
> 
> Cc: Andrew Fish afish@apple.com
> Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
> Cc: Bret Barkelew bret.barkelew@microsoft.com
> Cc: Brian J. Johnson brian.johnson@hpe.com
> Cc: Chasel Chiu chasel.chiu@intel.com
> Cc: Jordan Justen jordan.l.justen@intel.com
> Cc: Laszlo Ersek lersek@redhat.com
> Cc: Leif Lindholm leif@nuviainc.com
> Cc: Liming Gao liming.gao@intel.com
> Cc: Marvin Häuser mhaeuser@outlook.de
> Cc: Michael D Kinney michael.d.kinney@intel.com
> Cc: Vincent Zimmer vincent.zimmer@intel.com
> Cc: Zhichao Gao zhichao.gao@intel.com
> Cc: Jiewen Yao jiewen.yao@intel.com
> Signed-off-by: Vitaly Cheptsov vit9696@protonmail.com
> 
> Michael D Kinney (1):
>   MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK
> unit test
> 
> Vitaly Cheptsov (1):
>   MdePkg: Fix SafeString performing assertions on
> runtime checks
> 
>  MdePkg/Include/Library/BaseLib.h              | 111 --
> ---------------
>  MdePkg/Library/BaseLib/SafeString.c           | 115 +-
> ----------------
>  .../UnitTest/Library/BaseLib/Base64UnitTest.c | 107
> ++++++++++++++++
>  3 files changed, 110 insertions(+), 223 deletions(-)
> 
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

[Vitaly Cheptsov]

[-- Attachment #1.1: Type: text/html, Size: 6950 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

Re: [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

[Vitaly Cheptsov]

[-- Attachment #1.1: Type: text/html, Size: 7569 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

Re: [edk2-devel] [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

[Liming Gao]

Ack-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
> Sent: Thursday, May 21, 2020 4:10 AM
> To: devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Bret Barkelew <bret.barkelew@microsoft.com>;
> Brian J . Johnson <brian.johnson@hpe.com>; Chiu, Chasel <chasel.chiu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif@nuviainc.com>; Gao, Liming <liming.gao@intel.com>; Marvin H?user
> <mhaeuser@outlook.de>; Zimmer, Vincent <vincent.zimmer@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>; Philippe Mathieu-Daude <philmd@redhat.com>
> Subject: [edk2-devel] [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test
> 
> Use the safe string function StrCpyS() in BaseLib to test the
> SAFE_STRING_CONSTRAINT_CHECK() macro.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Bret Barkelew <bret.barkelew@microsoft.com>
> Cc: Brian J. Johnson <brian.johnson@hpe.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Marvin H?user <mhaeuser@outlook.de>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Vincent Zimmer <vincent.zimmer@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
>  .../UnitTest/Library/BaseLib/Base64UnitTest.c | 107 ++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> index 8952f9da6c..2c4266491c 100644
> --- a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> +++ b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> @@ -290,6 +290,99 @@ RfcDecodeTest(
>    return UNIT_TEST_PASSED;
>  }
> 
> +#define SOURCE_STRING  L"Hello"
> +
> +STATIC
> +UNIT_TEST_STATUS
> +EFIAPI
> +SafeStringContraintCheckTest (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  RETURN_STATUS  Status;
> +  CHAR16         Destination[20];
> +  CHAR16         AllZero[20];
> +
> +  //
> +  // Zero buffer used to verify Destination is not modified
> +  //
> +  ZeroMem (AllZero, sizeof (AllZero));
> +
> +  //
> +  // Positive test case copy source unicode string to destination
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), SOURCE_STRING);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING, sizeof (SOURCE_STRING));
> +
> +  //
> +  // Positive test case with DestMax the same as Source size
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, sizeof (SOURCE_STRING) / sizeof (CHAR16), SOURCE_STRING);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING, sizeof (SOURCE_STRING));
> +
> +  //
> +  // Negative test case with Destination NULL
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (NULL, sizeof (Destination) / sizeof (CHAR16), SOURCE_STRING);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with Source NULL
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), NULL);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with DestMax too big
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, MAX_UINTN, SOURCE_STRING);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with DestMax 0
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, 0, SOURCE_STRING);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with DestMax smaller than Source size
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, 1, SOURCE_STRING);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with DestMax smaller than Source size by one character
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, sizeof (SOURCE_STRING) / sizeof (CHAR16) - 1, SOURCE_STRING);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  //
> +  // Negative test case with overlapping Destination and Source
> +  //
> +  ZeroMem (Destination, sizeof (Destination));
> +  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), Destination);
> +  UT_ASSERT_STATUS_EQUAL (Status, RETURN_ACCESS_DENIED);
> +  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
>  /**
>    Initialze the unit test framework, suite, and unit tests for the
>    Base64 conversion APIs of BaseLib and run the unit tests.
> @@ -309,6 +402,7 @@ UnitTestingEntry (
>    UNIT_TEST_FRAMEWORK_HANDLE  Fw;
>    UNIT_TEST_SUITE_HANDLE      b64EncodeTests;
>    UNIT_TEST_SUITE_HANDLE      b64DecodeTests;
> +  UNIT_TEST_SUITE_HANDLE      SafeStringTests;
> 
>    Fw = NULL;
> 
> @@ -367,6 +461,19 @@ UnitTestingEntry (
>    AddTestCase (b64DecodeTests, "Incorrectly placed padding character", "Error4", RfcDecodeTest, NULL, CleanUpB64TestContext,
> &mBasicDecodeError4);
>    AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", RfcDecodeTest, NULL, CleanUpB64TestContext,
> &mBasicDecodeError5);
> 
> +  //
> +  // Populate the safe string Unit Test Suite.
> +  //
> +  Status = CreateUnitTestSuite (&SafeStringTests, Fw, "Safe String", "BaseLib.SafeString", NULL, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for SafeStringTests\n"));
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto EXIT;
> +  }
> +
> +  // --------------Suite-----------Description--------------Class Name----------Function--------Pre---Post-------------------Context-----------
> +  AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", "SafeStringContraintCheckTest", SafeStringContraintCheckTest,
> NULL, NULL, NULL);
> +
>    //
>    // Execute the tests.
>    //
> --
> 2.21.0.windows.1
> 
> 
>