Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.

["Fu, Siyuan" <siyuan.fu@intel.com>]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>



> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, September 4, 2018 3:17 PM
> To: edk2-devel@lists.01.org
> Cc: Stephen Benjamin <stephen@redhat.com>; Laszlo Ersek
> <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value
> in HTTP header.
> 
> This patch is to resolve the lock-up issue if the value of HTTP header
> is blank.  The issue is recorded @
> https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
> 
> Cc: Stephen Benjamin <stephen@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
>  1 file changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 5fbb50d03a..2fc3da8a2d 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
>      return NULL;
>    }
> 
>    //
>    // Each header field consists of a name followed by a colon (":") and
> the field value.
> +  // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> +  //
> +  // message-header = field-name ":" [ field-value ]
> +  // field-name = token
> +  // field-value = *( field-content | LWS )
> +  //
> +  // Note: "*(element)" allows any number element, including zero;
> "1*(element)" requires at least one element.
> +  //       [element] means element is optional.
> +  //       LWS  = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or
> '\r\n\t'.
> +  //       CRLF = '\r\n'.
> +  //       SP   = ' '.
> +  //       HT   = '\t' (Tab).
>    //
>    FieldNameStr = String;
>    FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
>    if (FieldValueStr == NULL) {
>      return NULL;
>    }
> 
>    //
> -  // Replace ':' with 0
> +  // Replace ':' with 0, then FieldName has been retrived from String.
>    //
>    *(FieldValueStr - 1) = 0;
> 
>    //
> -  // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> -  // Note: LWS  = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' '
> or '\t'.
> -  //       CRLF = '\r\n'.
> -  //       SP   = ' '.
> -  //       HT   = '\t' (Tab).
> +  // Handle FieldValueStr, skip all the preceded LWS.
>    //
>    while (TRUE) {
>      if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
>        //
>        // Boundary condition check.
>        //
>        if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
> +        //
> +        // Wrong String format!
> +        //
>          return NULL;
>        }
> 
>        FieldValueStr ++;
>      } else if (*FieldValueStr == '\r') {
>        //
>        // Boundary condition check.
>        //
>        if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
> -        return NULL;
> +        //
> +        // No more preceded LWS, so break here.
> +        //
> +        break;
>        }
> 
> -      if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' ||
> *(FieldValueStr + 2) == '\t')) {
> -        FieldValueStr = FieldValueStr + 3;
> +      if (*(FieldValueStr + 1) == '\n' ) {
> +        if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')
> {
> +          FieldValueStr = FieldValueStr + 3;
> +        } else {
> +          //
> +          // No more preceded LWS, so break here.
> +          //
> +          break;
> +        }
> +      } else {
> +        //
> +        // Wrong String format!
> +        //
> +        return NULL;
>        }
>      } else {
> +      //
> +      // No more preceded LWS, so break here.
> +      //
>        break;
>      }
>    }
> 
> -  //
> -  // Header fields can be extended over multiple lines by preceding each
> extra
> -  // line with at least one SP or HT.
> -  //
>    StrPtr = FieldValueStr;
>    do {
> +    //
> +    // Handle the LWS within the field value.
> +    //
>      StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
>      if (StrPtr == NULL || *StrPtr != '\n') {
> +      //
> +      // Wrong String format!
> +      //
>        return NULL;
>      }
> 
>      StrPtr++;
>    } while (*StrPtr == ' ' || *StrPtr == '\t');
> --
> 2.17.1.windows.2