On Tue, Dec 26, 2017 at 10:36:47AM +0800, Jiaxin Wu wrote:
> v2:
> * Fix GCC the build error.
>
> This patch is to add the boundary condition check to make sure
> the accessed buffer is valid.
>
> Cc: Gary Lin <[email protected]>
> Cc: Ye Ting <[email protected]>
> Cc: Fu Siyuan <[email protected]>
> Cc: Wang Fan <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <[email protected]>
> ---
> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 38
> +++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index caddbb7..915b81d 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -54,11 +54,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 +1556,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 +1594,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.
Trailing whitespaces (nitpicking :-)
> + //
> + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) {
> + return NULL;
ditto
> + }
> +
ditto
> FieldValueStr ++;
> - } else if (*FieldValueStr == '\r' && *(FieldValueStr + 1) == '\n' &&
> - (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) ==
> '\t')) {
> - FieldValueStr = FieldValueStr + 3;
> + } else if (*FieldValueStr == '\r') {
> + //
> + // Boundary condition check.
ditto
> + //
> + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) {
> + return NULL;
ditto
> + }
> +
> + if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' ||
> *(FieldValueStr + 2) == '\t')) {
> + FieldValueStr = FieldValueStr + 3;
> + }
> } else {
> break;
> }
> }
>
This patch fixes the gcc build.
Tested-by: Gary Lin <[email protected]>
Gary Lin
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
