* [Patch v9 1/2] MdePkg: Fix SafeString performing assertions on runtime checks
2020-05-20 20:10 [Patch v9 0/2] Disable safe string constraint assertions Michael D Kinney
@ 2020-05-20 20:10 ` Michael D Kinney
2020-05-20 20:10 ` [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test Michael D Kinney
2020-05-20 20:13 ` [edk2-devel] [Patch v9 0/2] Disable safe string constraint assertions Michael D Kinney
2 siblings, 0 replies; 7+ messages in thread
From: Michael D Kinney @ 2020-05-20 20:10 UTC (permalink / raw)
To: devel
Cc: Vitaly Cheptsov, Andrew Fish, Ard Biesheuvel, Bret Barkelew,
Brian J . Johnson, Chasel Chiu, Jordan Justen, Laszlo Ersek,
Leif Lindholm, Liming Gao, Marvin H?user, Vincent Zimmer,
Zhichao Gao, Jiewen Yao
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test
2020-05-20 20:10 [Patch v9 0/2] Disable safe string constraint assertions Michael D Kinney
2020-05-20 20:10 ` [Patch v9 1/2] MdePkg: Fix SafeString performing assertions on runtime checks Michael D Kinney
@ 2020-05-20 20:10 ` Michael D Kinney
2020-05-21 4:24 ` Vitaly Cheptsov
` (2 more replies)
2020-05-20 20:13 ` [edk2-devel] [Patch v9 0/2] Disable safe string constraint assertions Michael D Kinney
2 siblings, 3 replies; 7+ messages in thread
From: Michael D Kinney @ 2020-05-20 20:10 UTC (permalink / raw)
To: devel
Cc: Andrew Fish, Ard Biesheuvel, Bret Barkelew, Brian J . Johnson,
Chasel Chiu, Jordan Justen, Laszlo Ersek, Leif Lindholm,
Liming Gao, Marvin H?user, Vincent Zimmer, Zhichao Gao,
Jiewen Yao, Vitaly Cheptsov, Philippe Mathieu-Daude
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
^ permalink raw reply related [flat|nested] 7+ messages in thread