Re: [edk2-devel] [PATCH 1/1] NetworkPkg: Fix possible infinite loop in HTTP msg body parser

["Maciej Rabeda" <maciej.rabeda@linux.intel.com>]

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 <vladimir.olovyannikov@broadcom.com>
>> Sent: Friday, August 28, 2020 11:17 AM
>> To: devel@edk2.groups.io
>> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Maciej
>> Rabeda <maciej.rabeda@linux.intel.com>; Jiaxin Wu <jiaxin.wu@intel.com>;
>> Siyuan Fu <siyuan.fu@intel.com>
>> 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
>> <vladimir.olovyannikov@broadcom.com>
>> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> ---
>>   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
>
> 
>
>