From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web11.11960.1601459813657365520 for ; Wed, 30 Sep 2020 02:56:53 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.115, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: cflAZugeHqJ1YgEv8RYMhRVY8Ezp94N/xytXbsKi4UyegURk1uTBgFxN18Pgl9eAGfGwKaaB1L sjKeJnxxlyEQ== X-IronPort-AV: E=McAfee;i="6000,8403,9759"; a="161629870" X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="161629870" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 02:56:52 -0700 IronPort-SDR: x3vbAyKvt//OjNNc8BzWJSDanwEQbDe8fIaB/rt8o9FXQ8Vhq8Z7ZX85rCcOy1G1IGydD7lWVm Sfc2031fJpfA== X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="496712113" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.213.31.93]) ([10.213.31.93]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 02:56:51 -0700 Subject: Re: [edk2-devel] [PATCH 1/1] NetworkPkg: Fix possible infinite loop in HTTP msg body parser To: devel@edk2.groups.io, vladimir.olovyannikov@broadcom.com Cc: Jiaxin Wu , Siyuan Fu , Laszlo Ersek References: <20200828181706.25296-1-vladimir.olovyannikov@broadcom.com> <2d7b8b14f01cc630017e3e1134f17585@mail.gmail.com> From: "Maciej Rabeda" Message-ID: Date: Wed, 30 Sep 2020 11:56:45 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <2d7b8b14f01cc630017e3e1134f17585@mail.gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: pl Hi Vladimir, Yes, this must have go past my radar, sorry. Things are becoming more and more busy out here :/ I will take a look at it by the end of week. On 24-Sep-20 23:57, Vladimir Olovyannikov via groups.io wrote: > Hi Maciej, > > Can you please review this patch? > It is sitting there for a while, looks like it slipped through the cracks. > > Thank you, > Vladimir >> -----Original Message----- >> From: Vladimir Olovyannikov >> Sent: Friday, August 28, 2020 11:17 AM >> To: devel@edk2.groups.io >> Cc: Vladimir Olovyannikov ; Maciej >> Rabeda ; Jiaxin Wu ; >> Siyuan Fu >> Subject: [PATCH 1/1] NetworkPkg: Fix possible infinite loop in HTTP msg > body >> parser >> >> When an HTTP server sends a non-chunked body data with no Content- >> Length header, the HttpParserMessageBody in DxeHttpLib gets confused >> and never sets the Char pointer beyond the body start. >> This causes "for" loop to never break because the condition of "Char >= > Body >> + BodyLength" is never satisfied. >> Use BodyLength as the ContentLength for the parser when ContentLength is >> absent in HTTP response headers. >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2941 >> >> Signed-off-by: Vladimir Olovyannikov >> >> Cc: Maciej Rabeda >> Cc: Jiaxin Wu >> Cc: Siyuan Fu >> --- >> NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c | 19 ++++++++++++++++--- >> 1 file changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c >> b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c >> index 180d9321025a..e550c9962dc1 100644 >> --- a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c >> +++ b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c >> @@ -1122,6 +1122,7 @@ HttpParseMessageBody ( >> CHAR8 *Char; >> UINTN RemainderLengthInThis; >> UINTN LengthForCallback; >> + UINTN PortionLength; >> EFI_STATUS Status; >> HTTP_BODY_PARSER *Parser; >> >> @@ -1173,19 +1174,31 @@ HttpParseMessageBody ( >> // >> // Identity transfer-coding, just notify user to save the body > data. >> // >> + PortionLength = MIN ( >> + BodyLength, >> + Parser->ContentLength - > Parser->ParsedBodyLength >> + ); >> + if (!PortionLength) { >> + // >> + // Got BodyLength, but no ContentLength. Use BodyLength. >> + // >> + PortionLength = BodyLength; >> + Parser->ContentLength = PortionLength; >> + } >> + >> if (Parser->Callback != NULL) { >> Status = Parser->Callback ( >> BodyParseEventOnData, >> Char, >> - MIN (BodyLength, Parser->ContentLength - > Parser- >>> ParsedBodyLength), >> + PortionLength, >> Parser->Context >> ); >> if (EFI_ERROR (Status)) { >> return Status; >> } >> } >> - Char += MIN (BodyLength, Parser->ContentLength - Parser- >>> ParsedBodyLength); >> - Parser->ParsedBodyLength += MIN (BodyLength, Parser- >>> ContentLength - Parser->ParsedBodyLength); >> + Char += PortionLength; >> + Parser->ParsedBodyLength += PortionLength; >> if (Parser->ParsedBodyLength == Parser->ContentLength) { >> Parser->State = BodyParserComplete; >> if (Parser->Callback != NULL) { >> -- >> 2.26.2.266.ge870325ee8 > > > >