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

["Vladimir Olovyannikov" <vladimir.olovyannikov@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 3444 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4193 bytes --]