From nobody Thu Apr 25 10:11:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+59994+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+59994+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1590005426; cv=none; d=zohomail.com; s=zohoarc; b=A/HLrTxw6hYzl2BVWLMtCfFmH8sm+ekGVpwKZ7ZGgkOqssenpJjgh3DbvXX5YT+VfRGWd4ea+xIIedRXwaLxjyaty8e8wCqNYIlwb7/yYuYezO8co7xrVzXxADkMxR91GJYiz5jDCGvag4H9TksVdhqkzSalx7uOJvd8UnG9Dc8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1590005426; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=44Zq9WDhoWYtuUprExahRyc54ajtcDg40q8QR+Krd7w=; b=JPUVA9B82D2dBMqIvPK2fhXwQGmbU32oq/kpLwkMMpgS4n19iqEi4CbThzy6mScGw2Dgya8PkR5NRGB8HmlFOnIbUnUr+WLUT/Vgj8jyREeGBbvLGpVhvBARDbjOlosn/pjUiPrmWDJv4ajZb3gx76mlIby6zepkj90dk3S0jQM= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+59994+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1590005426302662.9202596567862; Wed, 20 May 2020 13:10:26 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id Ky8vYY1788612xcZ6GlBveLJ; Wed, 20 May 2020 13:10:25 -0700 X-Received: from mga03.intel.com (mga03.intel.com []) by mx.groups.io with SMTP id smtpd.web11.746.1590005424695289668 for ; Wed, 20 May 2020 13:10:25 -0700 IronPort-SDR: GjsO4wb4msQblqxJzZcDPLBVc2CbLPsUvF/7kBRhdMVxcGcvgLIeHssg/0uFSAUUCKyACeImLp mfEzIF81wf/A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2020 13:10:25 -0700 IronPort-SDR: kN8XgVMz7H5JCYbFTuM3LbOfoftH1OEm1baccSOmawXK6RqdyuHJ1t8gpVEEFRpgfqg9vt7wyJ 3KYWWI7t7Prg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,415,1583222400"; d="scan'208";a="308822887" X-Received: from mdkinney-mobl2.amr.corp.intel.com ([10.254.67.110]) by FMSMGA003.fm.intel.com with ESMTP; 20 May 2020 13:10:23 -0700 From: "Michael D Kinney" To: devel@edk2.groups.io 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 Subject: [edk2-devel] [Patch v9 1/2] MdePkg: Fix SafeString performing assertions on runtime checks Date: Wed, 20 May 2020 13:10:21 -0700 Message-Id: <20200520201022.28196-2-michael.d.kinney@intel.com> In-Reply-To: <20200520201022.28196-1-michael.d.kinney@intel.com> References: <20200520201022.28196-1-michael.d.kinney@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com X-Gm-Message-State: VlHUHWs01rD3MxUcV6JLKYa7x1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1590005425; bh=+TYmQ6QJL5q5LG7SibqW8Jn71S2YrtL0OjslXBc6U6I=; h=Cc:Date:From:Reply-To:Subject:To; b=oBFaTU9G1/TvwA+8FHnucs0XmYsW24xuYYLjtjUodIoqVCY+n+A8rzJ3v5LlSmQUoLN i+0tXJJtONThEHGp8dYFf+CDtYnLI0WQU7Y2UTt2X47ITGHRIZTfwQI2X4gr0CU7l4Wls m5n8a3asNwM4pB9B2anWcei7xivmLvr0Gag= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" From: Vitaly Cheptsov REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054 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 Cc: Ard Biesheuvel Cc: Bret Barkelew Cc: Brian J. Johnson Cc: Chasel Chiu Cc: Jordan Justen Cc: Laszlo Ersek Cc: Leif Lindholm Cc: Liming Gao Cc: Marvin H?user Cc: Michael D Kinney Cc: Vincent Zimmer Cc: Zhichao Gao Cc: Jiewen Yao Signed-off-by: Vitaly Cheptsov Reviewed-by: Liming Gao Reviewed-by: Laszlo Ersek Reviewed-by: Bret Barkelew --- 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/Base= Lib.h index b0bbe8cef8..8e7b87cbda 100644 --- a/MdePkg/Include/Library/BaseLib.h +++ b/MdePkg/Include/Library/BaseLib.h @@ -216,7 +216,6 @@ StrnSizeS ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -252,7 +251,6 @@ StrCpyS ( =20 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 ASSER= T(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -290,7 +288,6 @@ StrnCpyS ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -330,7 +327,6 @@ StrCatS ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -377,12 +373,7 @@ StrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 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. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 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 ( =20 This function is similar as strcpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -683,8 +657,6 @@ AsciiStrCpyS ( =20 This function is similar as strncpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -719,8 +691,6 @@ AsciiStrnCpyS ( =20 This function is similar as strcat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -757,8 +727,6 @@ AsciiStrCatS ( =20 This function is similar as strncat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -804,12 +772,6 @@ AsciiStrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 sto= red 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. =20 - 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. =20 - 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 onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - If String is NULL, then ASSERT(). - If Guid is NULL, then ASSERT(). If String is not aligned in a 16-bit boundary, then ASSERT(). =20 @param String Pointer to a Null-terminated Unicode st= ring. @@ -1703,17 +1629,6 @@ StrToGuid ( =20 If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - 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 st= ring. @param Length The number of Unicode characters to dec= ode. @param Buffer Pointer to the converted bytes array. @@ -1804,7 +1719,6 @@ UnicodeStrToAsciiStr ( the upper 8 bits, then ASSERT(). =20 If Source is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -2415,10 +2328,6 @@ AsciiStrHexToUint64 ( "::" can be used to compress one or more groups of X when X contains onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - If String is NULL, then ASSERT(). - If Guid is NULL, then ASSERT(). - @param String Pointer to a Null-terminated ASCII stri= ng. @param Guid Pointer to the converted GUID. =20 @@ -2568,17 +2470,6 @@ AsciiStrToGuid ( decoding stops after Length of characters and outputs Buffer containing (Length / 2) bytes. =20 - 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 stri= ng. @param Length The number of ASCII characters to decod= e. @param Buffer Pointer to the converted bytes array. @@ -2659,7 +2550,6 @@ AsciiStrToUnicodeStr ( equal or greater than ((AsciiStrLen (Source) + 1) * sizeof (CHAR16)) in = bytes. =20 If Destination is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -2705,7 +2595,6 @@ AsciiStrToUnicodeStrS ( ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes. =20 If Destination is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then Destination and DestinationLength are unmodified. diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/S= afeString.c index 7dc03d2caa..3bb23ca1a1 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -14,8 +14,10 @@ =20 #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 ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -279,7 +280,6 @@ StrCpyS ( =20 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 ASSER= T(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -372,7 +372,6 @@ StrnCpyS ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -473,7 +472,6 @@ StrCatS ( =20 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(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -590,12 +588,7 @@ StrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 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. =20 - 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 t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). =20 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 onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han - PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - If String is NULL, then ASSERT(). - If Guid is NULL, then ASSERT(). If String is not aligned in a 16-bit boundary, then ASSERT(). =20 @param String Pointer to a Null-terminated Unicode st= ring. @@ -1589,17 +1549,6 @@ StrToGuid ( =20 If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - 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 st= ring. @param Length The number of Unicode characters to dec= ode. @param Buffer Pointer to the converted bytes array. @@ -1779,8 +1728,6 @@ AsciiStrnSizeS ( =20 This function is similar as strcpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -1856,8 +1803,6 @@ AsciiStrCpyS ( =20 This function is similar as strncpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -1944,8 +1889,6 @@ AsciiStrnCpyS ( =20 This function is similar as strcat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -2040,8 +1983,6 @@ AsciiStrCatS ( =20 This function is similar as strncat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). - If an error is returned, then the Destination is unmodified. =20 @param Destination A pointer to a Null-terminated Ascii st= ring. @@ -2154,12 +2095,6 @@ AsciiStrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 sto= red 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. =20 - 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. =20 - 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(). =20 If Source is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -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(). =20 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. =20 If Destination is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -2948,7 +2862,6 @@ AsciiStrToUnicodeStrS ( ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes. =20 If Destination is not aligned on a 16-bit boundary, then ASSERT(). - If an error would be returned, then the function will also ASSERT(). =20 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 onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). - - If Address is NULL, then ASSERT(). - If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - If String is NULL, then ASSERT(). - If Guid is NULL, then ASSERT(). - @param String Pointer to a Null-terminated ASCII stri= ng. @param Guid Pointer to the converted GUID. =20 @@ -3550,17 +3452,6 @@ AsciiStrToGuid ( decoding stops after Length of characters and outputs Buffer containing (Length / 2) bytes. =20 - 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 stri= ng. @param Length The number of ASCII characters to decod= e. @param Buffer Pointer to the converted bytes array. --=20 2.21.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59994): https://edk2.groups.io/g/devel/message/59994 Mute This Topic: https://groups.io/mt/74359141/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- From nobody Thu Apr 25 10:11:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+59995+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+59995+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1590005428; cv=none; d=zohomail.com; s=zohoarc; b=OsfzNnS2tLvHY3V0mafBYpNYQ7ED01GgeuY8q2kUQ7VYT/PqlhXUoTdpZ1gAF0if4tfHPBTWvCILF6s37USaGdyvO3w+BSFTWpFXc61vb0UQrUPsJhodiAeSJemi6frD+HxJZdn4wbJ1Sih0sfajmyQRrZVhv080iwvNjvi2H6s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1590005428; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=3nSIVi7bGAjASlXO6tnGe8KTC7FE9qluP+gAI1eAsbI=; b=kNlGvNfExmZlIx8/CVuzMENV1Hjg9I7B8KiWmFBoAgUUqi7YqW0NINtF88fbNOVPGnTI7HkkAS2L3NikBs88BWx8Np7SdtQ8VELNfYEaGOq3ANXqs+rCQMJHZHNc6069dIlcIa1pAa5r2XLcCpOf+PEt15VTbhiF0rXQ9EIm5CA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+59995+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1590005428133372.8553369470085; Wed, 20 May 2020 13:10:28 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id fG9WYY1788612xHLlT211m1x; Wed, 20 May 2020 13:10:26 -0700 X-Received: from mga03.intel.com (mga03.intel.com []) by mx.groups.io with SMTP id smtpd.web11.746.1590005424695289668 for ; Wed, 20 May 2020 13:10:26 -0700 IronPort-SDR: YwVZVQaGaEub2VLD6+UftqIfnxPXUwh6ko8lL2dWAJk7HqpmOxJfPunFpYGQ/I+uf7RPzckqUo UbbReF6nBrtA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2020 13:10:26 -0700 IronPort-SDR: 7cmtV1O6DQCLi3VSa4peh2zTgWAxMRXutToNJFoM+4Xv4sd34J3HyzALHB8DXbAhnwaRwvv2KZ HXDQ5VkP+cgQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,415,1583222400"; d="scan'208";a="308822892" X-Received: from mdkinney-mobl2.amr.corp.intel.com ([10.254.67.110]) by FMSMGA003.fm.intel.com with ESMTP; 20 May 2020 13:10:25 -0700 From: "Michael D Kinney" To: devel@edk2.groups.io 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 Subject: [edk2-devel] [Patch v9 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test Date: Wed, 20 May 2020 13:10:22 -0700 Message-Id: <20200520201022.28196-3-michael.d.kinney@intel.com> In-Reply-To: <20200520201022.28196-1-michael.d.kinney@intel.com> References: <20200520201022.28196-1-michael.d.kinney@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com X-Gm-Message-State: 8DbGFfBiceBnyEWAwx8PV3jrx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1590005426; bh=R/MZupK8TuiY1twqrd9+9wST3xbnIEd7ZJRAeqrefGo=; h=Cc:Date:From:Reply-To:Subject:To; b=t5ZACzhxl/FDnJL3BvHfC17gy6s9jE0CBIMcG9aOSbDAyGY+yaiyVNz4aZlJ+8yYY3Z o2s2kQ8QBCISTnz1xZu2TQ8ljqm0sYrfZ4jAaC95twbWLqLfoTd0vfX0FW0sFHED0e2kz AJMrmydUKDmfmH/vfDiyHWlqhez0Nc4xhHY= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" Use the safe string function StrCpyS() in BaseLib to test the SAFE_STRING_CONSTRAINT_CHECK() macro. Cc: Andrew Fish Cc: Ard Biesheuvel Cc: Bret Barkelew Cc: Brian J. Johnson Cc: Chasel Chiu Cc: Jordan Justen Cc: Laszlo Ersek Cc: Leif Lindholm Cc: Liming Gao Cc: Marvin H?user Cc: Michael D Kinney Cc: Vincent Zimmer Cc: Zhichao Gao Cc: Jiewen Yao Cc: Vitaly Cheptsov Signed-off-by: Michael D Kinney Reviewed-by: Philippe Mathieu-Daude --- .../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; } =20 +#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 =3D 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 =3D 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 =3D 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 =3D 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 =3D 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 =3D 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 =3D 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 chara= cter + // + ZeroMem (Destination, sizeof (Destination)); + Status =3D 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 =3D 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; =20 Fw =3D NULL; =20 @@ -367,6 +461,19 @@ UnitTestingEntry ( AddTestCase (b64DecodeTests, "Incorrectly placed padding character", "Er= ror4", RfcDecodeTest, NULL, CleanUpB64TestContext, &mBasicDecodeError4); AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", Rfc= DecodeTest, NULL, CleanUpB64TestContext, &mBasicDecodeError5); =20 + // + // Populate the safe string Unit Test Suite. + // + Status =3D CreateUnitTestSuite (&SafeStringTests, Fw, "Safe String", "Ba= seLib.SafeString", NULL, NULL); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for SafeStringTest= s\n")); + Status =3D EFI_OUT_OF_RESOURCES; + goto EXIT; + } + + // --------------Suite-----------Description--------------Class Name----= ------Function--------Pre---Post-------------------Context----------- + AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", "SafeStrin= gContraintCheckTest", SafeStringContraintCheckTest, NULL, NULL, NULL); + // // Execute the tests. // --=20 2.21.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59995): https://edk2.groups.io/g/devel/message/59995 Mute This Topic: https://groups.io/mt/74359142/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-