From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jiaxin.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 3A69D22225C13 for ; Mon, 25 Dec 2017 18:16:19 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Dec 2017 18:21:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,457,1508828400"; d="scan'208";a="16531200" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga001.fm.intel.com with ESMTP; 25 Dec 2017 18:21:13 -0800 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 25 Dec 2017 18:21:13 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 25 Dec 2017 18:21:12 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.218]) with mapi id 14.03.0319.002; Tue, 26 Dec 2017 10:21:10 +0800 From: "Wu, Jiaxin" To: Gary Lin CC: "edk2-devel@lists.01.org" , "Ye, Ting" , "Wang, Fan" , "Fu, Siyuan" Thread-Topic: [edk2] [Patch 1/5] MdeModulePkg/DxeHttpLib: Add boundary condition check. Thread-Index: AQHTfemZv0GubW0nFkyA2CAK29mMIqNUV2+AgACM9cA= Date: Tue, 26 Dec 2017 02:21:10 +0000 Message-ID: <895558F6EA4E3B41AC93A00D163B727416356C9B@SHSMSX103.ccr.corp.intel.com> References: <1514252029-12720-1-git-send-email-jiaxin.wu@intel.com> <1514252029-12720-2-git-send-email-jiaxin.wu@intel.com> <20171226015603.c22tfqq6vj5oesc7@GaryWorkstation> In-Reply-To: <20171226015603.c22tfqq6vj5oesc7@GaryWorkstation> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDZlZWIwNDEtMmI1Ni00MzQ4LThlMDgtODhmOWQ3Y2M0YzM4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IndIdjV2ekZucFIyeXE3WGdkSXZNTE1hZ09UcXpiRDhKXC8xOWI5SkJJVitrPSJ9 x-ctpclassification: CTP_IC 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 1/5] MdeModulePkg/DxeHttpLib: Add boundary condition check. 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: Tue, 26 Dec 2017 02:16:20 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks to catch that. Best Regards! Jiaxin > -----Original Message----- > From: Gary Lin [mailto:glin@suse.com] > Sent: Tuesday, December 26, 2017 9:56 AM > To: Wu, Jiaxin > Cc: edk2-devel@lists.01.org; Ye, Ting ; Wang, Fan > ; Fu, Siyuan > Subject: Re: [edk2] [Patch 1/5] MdeModulePkg/DxeHttpLib: Add boundary > condition check. >=20 > On Tue, Dec 26, 2017 at 09:33:45AM +0800, Jiaxin Wu wrote: > > This patch is to add the boundary condition check to make sure > > the accessed buffer is valid. > > > > Cc: Ye Ting > > Cc: Fu Siyuan > > Cc: Wang Fan > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Wu Jiaxin > > --- > > MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 39 > ++++++++++++++++++++++++---- > > 1 file changed, 34 insertions(+), 5 deletions(-) > > > > diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c > b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c > > index caddbb7..4d353d7 100644 > > --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c > > +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c > > @@ -33,11 +33,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > > @retval EFI_SUCCESS Successfully decoded the URI. > > @retval EFI_INVALID_PARAMETER Buffer is not a valid percent-encoded > string. > > > > **/ > > EFI_STATUS > > -EFIAPI > > UriPercentDecode ( > > IN CHAR8 *Buffer, > > IN UINT32 BufferLength, > > OUT CHAR8 *ResultBuffer, > > OUT UINT32 *ResultLength > This change will break gcc build since EFIAPI is still declared in > HttpLib.h. >=20 > Gary Lin >=20 > > @@ -54,11 +53,11 @@ UriPercentDecode ( > > Index =3D 0; > > Offset =3D 0; > > HexStr[2] =3D '\0'; > > while (Index < BufferLength) { > > if (Buffer[Index] =3D=3D '%') { > > - if (!NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR > (Buffer[Index+2])) { > > + if (Index + 1 >=3D BufferLength || Index + 2 >=3D BufferLength > || !NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR > (Buffer[Index+2])) { > > return EFI_INVALID_PARAMETER; > > } > > HexStr[0] =3D Buffer[Index+1]; > > HexStr[1] =3D Buffer[Index+2]; > > ResultBuffer[Offset] =3D (CHAR8) AsciiStrHexToUintn (HexStr); > > @@ -1556,20 +1555,31 @@ HttpGetFieldNameAndValue ( > > ) > > { > > CHAR8 *FieldNameStr; > > CHAR8 *FieldValueStr; > > CHAR8 *StrPtr; > > + CHAR8 *EndofHeader; > > > > if (String =3D=3D NULL || FieldName =3D=3D NULL || FieldValue =3D=3D= NULL) { > > return NULL; > > } > > > > *FieldName =3D NULL; > > *FieldValue =3D NULL; > > FieldNameStr =3D NULL; > > FieldValueStr =3D NULL; > > StrPtr =3D NULL; > > + EndofHeader =3D NULL; > > + > > + > > + // > > + // Check whether the raw HTTP header string is valid or not. > > + // > > + EndofHeader =3D AsciiStrStr (String, "\r\n\r\n"); > > + if (EndofHeader =3D=3D NULL) { > > + return NULL; > > + } > > > > // > > // Each header field consists of a name followed by a colon (":") an= d the > field value. > > // > > FieldNameStr =3D String; > > @@ -1583,17 +1593,36 @@ HttpGetFieldNameAndValue ( > > // > > *(FieldValueStr - 1) =3D 0; > > > > // > > // The field value MAY be preceded by any amount of LWS, though a > single SP is preferred. > > + // Note: LWS =3D [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or= ' ' or '\t'. > > + // CRLF =3D '\r\n'. > > + // SP =3D ' '. > > + // HT =3D '\t' (Tab). > > // > > while (TRUE) { > > if (*FieldValueStr =3D=3D ' ' || *FieldValueStr =3D=3D '\t') { > > + // > > + // Boundary condition check. > > + // > > + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) { > > + return NULL; > > + } > > + > > FieldValueStr ++; > > - } else if (*FieldValueStr =3D=3D '\r' && *(FieldValueStr + 1) =3D= =3D '\n' && > > - (*(FieldValueStr + 2) =3D=3D ' ' || *(FieldValueStr + 2= ) =3D=3D '\t')) { > > - FieldValueStr =3D FieldValueStr + 3; > > + } else if (*FieldValueStr =3D=3D '\r') { > > + // > > + // Boundary condition check. > > + // > > + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) { > > + return NULL; > > + } > > + > > + if (*(FieldValueStr + 1) =3D=3D '\n' && (*(FieldValueStr + 2) = =3D=3D ' ' || > *(FieldValueStr + 2) =3D=3D '\t')) { > > + FieldValueStr =3D FieldValueStr + 3; > > + } > > } else { > > break; > > } > > } > > > > -- > > 1.9.5.msysgit.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > >