Thanks to catch that. Best Regards! Jiaxin
> -----Original Message----- > From: Gary Lin [mailto:g...@suse.com] > Sent: Tuesday, December 26, 2017 9:56 AM > To: Wu, Jiaxin <jiaxin...@intel.com> > Cc: edk2-devel@lists.01.org; Ye, Ting <ting...@intel.com>; Wang, Fan > <fan.w...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > Subject: Re: [edk2] [Patch 1/5] MdeModulePkg/DxeHttpLib: Add boundary > condition check. > > 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 <ting...@intel.com> > > Cc: Fu Siyuan <siyuan...@intel.com> > > Cc: Wang Fan <fan.w...@intel.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Wu Jiaxin <jiaxin...@intel.com> > > --- > > 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. > > Gary Lin > > > @@ -54,11 +53,11 @@ UriPercentDecode ( > > Index = 0; > > Offset = 0; > > HexStr[2] = '\0'; > > while (Index < BufferLength) { > > if (Buffer[Index] == '%') { > > - if (!NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR > (Buffer[Index+2])) { > > + if (Index + 1 >= BufferLength || Index + 2 >= BufferLength > || !NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR > (Buffer[Index+2])) { > > return EFI_INVALID_PARAMETER; > > } > > HexStr[0] = Buffer[Index+1]; > > HexStr[1] = Buffer[Index+2]; > > ResultBuffer[Offset] = (CHAR8) AsciiStrHexToUintn (HexStr); > > @@ -1556,20 +1555,31 @@ HttpGetFieldNameAndValue ( > > ) > > { > > CHAR8 *FieldNameStr; > > CHAR8 *FieldValueStr; > > CHAR8 *StrPtr; > > + CHAR8 *EndofHeader; > > > > if (String == NULL || FieldName == NULL || FieldValue == NULL) { > > return NULL; > > } > > > > *FieldName = NULL; > > *FieldValue = NULL; > > FieldNameStr = NULL; > > FieldValueStr = NULL; > > StrPtr = NULL; > > + EndofHeader = NULL; > > + > > + > > + // > > + // Check whether the raw HTTP header string is valid or not. > > + // > > + EndofHeader = AsciiStrStr (String, "\r\n\r\n"); > > + if (EndofHeader == NULL) { > > + return NULL; > > + } > > > > // > > // Each header field consists of a name followed by a colon (":") and the > field value. > > // > > FieldNameStr = String; > > @@ -1583,17 +1593,36 @@ HttpGetFieldNameAndValue ( > > // > > *(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). > > // > > while (TRUE) { > > if (*FieldValueStr == ' ' || *FieldValueStr == '\t') { > > + // > > + // Boundary condition check. > > + // > > + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) { > > + return NULL; > > + } > > + > > FieldValueStr ++; > > - } else if (*FieldValueStr == '\r' && *(FieldValueStr + 1) == '\n' && > > - (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == > > '\t')) { > > - FieldValueStr = FieldValueStr + 3; > > + } else if (*FieldValueStr == '\r') { > > + // > > + // Boundary condition check. > > + // > > + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) { > > + return NULL; > > + } > > + > > + if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || > *(FieldValueStr + 2) == '\t')) { > > + FieldValueStr = 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 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
