From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D58512034A8A6 for ; Mon, 5 Feb 2018 00:37:18 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Feb 2018 00:43:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,464,1511856000"; d="scan'208";a="27334640" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 05 Feb 2018 00:42:59 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 5 Feb 2018 00:42:57 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.124]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.253]) with mapi id 14.03.0319.002; Mon, 5 Feb 2018 16:42:11 +0800 From: "Yao, Jiewen" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: "Gao, Liming" , "Wang, Jian J" Thread-Topic: [PATCH v2] MdePkg/SafeString: Fix potential out-of-bound memory access Thread-Index: AQHTnkHZ11zbLIY2TkiRvvT0dbyOtaOVfd3A Date: Mon, 5 Feb 2018 08:42:10 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AAB4184@shsmsx102.ccr.corp.intel.com> References: <20180205052610.203088-1-ruiyu.ni@intel.com> In-Reply-To: <20180205052610.203088-1-ruiyu.ni@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWU2NzBjZTAtYTA4Ny00YWVjLWE1YjgtMTA3ZmU1NGY1ODUxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJ3cVlVSloyd3F1ZXFkckl6OHJrTkpaVjFlaUFBOXg3UEl3dk1BR0RxOU9YekZcL3RVTFRyN2pIekcxOEQrQTFXKyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] MdePkg/SafeString: Fix potential out-of-bound memory access X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Feb 2018 08:37:20 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jiewen.yao@intel.com > -----Original Message----- > From: Ni, Ruiyu > Sent: Monday, February 5, 2018 1:26 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Gao, Liming ; > Wang, Jian J > Subject: [PATCH v2] MdePkg/SafeString: Fix potential out-of-bound memory > access >=20 > Today's implementation of [Ascii]StrnCpyS/[Ascii]StrnCatS calls > StrnLenS () to get the length of source string but supplies the > destination buffer size as max size. > It's a bug that may cause out-of-bound memory access. > For example: > StrnCpyS (Dest[10], 10, "hello", 6) > -> StrnLenS ("hello", 10) //< cause out-of bound memory access >=20 > In a pool guard enabled environment, when using shell to edit an > existing file which contains empty line, the page fault is met. >=20 > The patch fixes the four library functions to avoid such > out-of-bound memory access. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni > Cc: Jiewen Yao > Cc: Liming Gao > Cc: Jian J Wang > --- > MdePkg/Library/BaseLib/SafeString.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) >=20 > diff --git a/MdePkg/Library/BaseLib/SafeString.c > b/MdePkg/Library/BaseLib/SafeString.c > index 68c33e9b7b..29310889ca 100644 > --- a/MdePkg/Library/BaseLib/SafeString.c > +++ b/MdePkg/Library/BaseLib/SafeString.c > @@ -1,7 +1,7 @@ > /** @file > Safe String functions. >=20 > - Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.
> + Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at > @@ -342,7 +342,7 @@ StrnCpyS ( > // > // 4. If Length is not less than DestMax, then DestMax shall be greate= r than > StrnLenS(Source, DestMax). > // > - SourceLen =3D StrnLenS (Source, DestMax); > + SourceLen =3D StrnLenS (Source, MIN (DestMax, Length)); > if (Length >=3D DestMax) { > SAFE_STRING_CONSTRAINT_CHECK ((DestMax > SourceLen), > RETURN_BUFFER_TOO_SMALL); > } > @@ -361,7 +361,7 @@ StrnCpyS ( > // pointed to by Destination. If no null character was copied from Sou= rce, > then Destination[Length] is set to a null > // character. > // > - while ((*Source !=3D 0) && (SourceLen > 0)) { > + while ((SourceLen > 0) && (*Source !=3D 0)) { > *(Destination++) =3D *(Source++); > SourceLen--; > } > @@ -551,7 +551,7 @@ StrnCatS ( > // > // 5. If Length is not less than CopyLen, then CopyLen shall be greate= r than > StrnLenS(Source, CopyLen). > // > - SourceLen =3D StrnLenS (Source, CopyLen); > + SourceLen =3D StrnLenS (Source, MIN (CopyLen, Length)); > if (Length >=3D CopyLen) { > SAFE_STRING_CONSTRAINT_CHECK ((CopyLen > SourceLen), > RETURN_BUFFER_TOO_SMALL); > } > @@ -572,7 +572,7 @@ StrnCatS ( > // a null character. > // > Destination =3D Destination + DestLen; > - while ((*Source !=3D 0) && (SourceLen > 0)) { > + while ((SourceLen > 0) && (*Source !=3D 0)) { > *(Destination++) =3D *(Source++); > SourceLen--; > } > @@ -1916,7 +1916,7 @@ AsciiStrnCpyS ( > // > // 4. If Length is not less than DestMax, then DestMax shall be greate= r than > AsciiStrnLenS(Source, DestMax). > // > - SourceLen =3D AsciiStrnLenS (Source, DestMax); > + SourceLen =3D AsciiStrnLenS (Source, MIN (DestMax, Length)); > if (Length >=3D DestMax) { > SAFE_STRING_CONSTRAINT_CHECK ((DestMax > SourceLen), > RETURN_BUFFER_TOO_SMALL); > } > @@ -1935,7 +1935,7 @@ AsciiStrnCpyS ( > // pointed to by Destination. If no null character was copied from Sou= rce, > then Destination[Length] is set to a null > // character. > // > - while ((*Source !=3D 0) && (SourceLen > 0)) { > + while ((SourceLen > 0) && (*Source !=3D 0)) { > *(Destination++) =3D *(Source++); > SourceLen--; > } > @@ -2115,7 +2115,7 @@ AsciiStrnCatS ( > // > // 5. If Length is not less than CopyLen, then CopyLen shall be greate= r than > AsciiStrnLenS(Source, CopyLen). > // > - SourceLen =3D AsciiStrnLenS (Source, CopyLen); > + SourceLen =3D AsciiStrnLenS (Source, MIN (CopyLen, Length)); > if (Length >=3D CopyLen) { > SAFE_STRING_CONSTRAINT_CHECK ((CopyLen > SourceLen), > RETURN_BUFFER_TOO_SMALL); > } > @@ -2136,7 +2136,7 @@ AsciiStrnCatS ( > // a null character. > // > Destination =3D Destination + DestLen; > - while ((*Source !=3D 0) && (SourceLen > 0)) { > + while ((SourceLen > 0) && (*Source !=3D 0)) { > *(Destination++) =3D *(Source++); > SourceLen--; > } > -- > 2.16.1.windows.1