From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=jiaxin.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EE1322096F32A for ; Sun, 1 Jul 2018 19:19:34 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Jul 2018 19:19:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,297,1526367600"; d="scan'208";a="242203855" Received: from jiaxinwu-mobl.ccr.corp.intel.com ([10.239.192.123]) by fmsmga006.fm.intel.com with ESMTP; 01 Jul 2018 19:19:33 -0700 From: Jiaxin Wu To: edk2-devel@lists.01.org Cc: Ye Ting , Fu Siyuan , Wu Jiaxin Date: Mon, 2 Jul 2018 10:19:30 +0800 Message-Id: <20180702021930.11488-1-Jiaxin.wu@intel.com> X-Mailer: git-send-email 2.17.1.windows.2 Subject: [Patch v2] NetworkPkg/HttpDxe: Fix the bug when parsing HTTP(S) message body. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 02:19:35 -0000 *v2: Resolve the conflict commit. HttpBodyParserCallback function is to parse the HTTP(S) message body so as to confirm whether there is the next message header. But it doesn't record the parsing message data/length correctly. This patch is refine the parsing logic so as to fix the potential failure. Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin Reviewed-by: Fu Siyuan --- NetworkPkg/HttpDxe/HttpImpl.c | 112 +++++++++++++++++---------------- NetworkPkg/HttpDxe/HttpProto.c | 10 +++ NetworkPkg/HttpDxe/HttpProto.h | 10 +++ 3 files changed, 77 insertions(+), 55 deletions(-) diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index f70e116f38..57a3712c23 100644 --- a/NetworkPkg/HttpDxe/HttpImpl.c +++ b/NetworkPkg/HttpDxe/HttpImpl.c @@ -914,10 +914,11 @@ HttpBodyParserCallback ( IN CHAR8 *Data, IN UINTN Length, IN VOID *Context ) { + HTTP_CALLBACK_DATA *CallbackData; HTTP_TOKEN_WRAP *Wrap; UINTN BodyLength; CHAR8 *Body; if (EventType != BodyParseEventOnComplete) { @@ -926,25 +927,22 @@ HttpBodyParserCallback ( if (Data == NULL || Length != 0 || Context == NULL) { return EFI_SUCCESS; } - Wrap = (HTTP_TOKEN_WRAP *) Context; - Body = Wrap->HttpToken->Message->Body; - BodyLength = Wrap->HttpToken->Message->BodyLength; + CallbackData = (HTTP_CALLBACK_DATA *) Context; + + Wrap = (HTTP_TOKEN_WRAP *) (CallbackData->Wrap); + Body = CallbackData->ParseData; + BodyLength = CallbackData->ParseDataLength; + if (Data < Body + BodyLength) { Wrap->HttpInstance->NextMsg = Data; } else { Wrap->HttpInstance->NextMsg = NULL; } - - // - // Free Tx4Token or Tx6Token since already received corrsponding HTTP response. - // - FreePool (Wrap); - return EFI_SUCCESS; } /** The work function of EfiHttpResponse(). @@ -1189,33 +1187,43 @@ HttpResponseWorker ( HttpInstance->Method, HttpMsg->Data.Response->StatusCode, HttpMsg->HeaderCount, HttpMsg->Headers, HttpBodyParserCallback, - (VOID *) ValueInItem, + (VOID *) (&HttpInstance->CallbackData), &HttpInstance->MsgParser ); if (EFI_ERROR (Status)) { goto Error2; } // // Check whether we received a complete HTTP message. // if (HttpInstance->CacheBody != NULL) { + // + // Record the CallbackData data. + // + HttpInstance->CallbackData.Wrap = (VOID *) Wrap; + HttpInstance->CallbackData.ParseData = (VOID *) HttpInstance->CacheBody; + HttpInstance->CallbackData.ParseDataLength = HttpInstance->CacheLen; + + // + // Parse message with CallbackData data. + // Status = HttpParseMessageBody (HttpInstance->MsgParser, HttpInstance->CacheLen, HttpInstance->CacheBody); if (EFI_ERROR (Status)) { goto Error2; } + } - if (HttpIsMessageComplete (HttpInstance->MsgParser)) { - // - // Free the MsgParse since we already have a full HTTP message. - // - HttpFreeMsgParser (HttpInstance->MsgParser); - HttpInstance->MsgParser = NULL; - } + if (HttpIsMessageComplete (HttpInstance->MsgParser)) { + // + // Free the MsgParse since we already have a full HTTP message. + // + HttpFreeMsgParser (HttpInstance->MsgParser); + HttpInstance->MsgParser = NULL; } } if ((HttpMsg->Body == NULL) || (HttpMsg->BodyLength == 0)) { Status = EFI_SUCCESS; @@ -1330,16 +1338,30 @@ HttpResponseWorker ( if (EFI_ERROR (Status)) { goto Error2; } // - // Check whether we receive a complete HTTP message. + // Process the received the body packet. + // + HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength); + + CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength); + + // + // Record the CallbackData data. + // + HttpInstance->CallbackData.Wrap = (VOID *) Wrap; + HttpInstance->CallbackData.ParseData = HttpMsg->Body; + HttpInstance->CallbackData.ParseDataLength = HttpMsg->BodyLength; + + // + // Parse Body with CallbackData data. // Status = HttpParseMessageBody ( HttpInstance->MsgParser, - (UINTN) Fragment.Len, - (CHAR8 *) Fragment.Bulk + HttpMsg->BodyLength, + HttpMsg->Body ); if (EFI_ERROR (Status)) { goto Error2; } @@ -1350,51 +1372,31 @@ HttpResponseWorker ( HttpFreeMsgParser (HttpInstance->MsgParser); HttpInstance->MsgParser = NULL; } // - // We receive part of header of next HTTP msg. + // Check whether there is the next message header in the HttpMsg->Body. // if (HttpInstance->NextMsg != NULL) { - HttpMsg->BodyLength = MIN ((UINTN) HttpInstance->NextMsg - (UINTN) Fragment.Bulk, HttpMsg->BodyLength); - CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength); - - HttpInstance->CacheLen = Fragment.Len - HttpMsg->BodyLength; - if (HttpInstance->CacheLen != 0) { - if (HttpInstance->CacheBody != NULL) { - FreePool (HttpInstance->CacheBody); - } - - HttpInstance->CacheBody = AllocateZeroPool (HttpInstance->CacheLen); - if (HttpInstance->CacheBody == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto Error2; - } - - CopyMem (HttpInstance->CacheBody, Fragment.Bulk + HttpMsg->BodyLength, HttpInstance->CacheLen); - HttpInstance->CacheOffset = 0; + HttpMsg->BodyLength = HttpInstance->NextMsg - (CHAR8 *) HttpMsg->Body; + } - HttpInstance->NextMsg = HttpInstance->CacheBody + ((UINTN) HttpInstance->NextMsg - (UINTN) (Fragment.Bulk + HttpMsg->BodyLength)); + HttpInstance->CacheLen = Fragment.Len - HttpMsg->BodyLength; + if (HttpInstance->CacheLen != 0) { + if (HttpInstance->CacheBody != NULL) { + FreePool (HttpInstance->CacheBody); } - } else { - HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength); - CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength); - HttpInstance->CacheLen = Fragment.Len - HttpMsg->BodyLength; - if (HttpInstance->CacheLen != 0) { - if (HttpInstance->CacheBody != NULL) { - FreePool (HttpInstance->CacheBody); - } - - HttpInstance->CacheBody = AllocateZeroPool (HttpInstance->CacheLen); - if (HttpInstance->CacheBody == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto Error2; - } - CopyMem (HttpInstance->CacheBody, Fragment.Bulk + HttpMsg->BodyLength, HttpInstance->CacheLen); - HttpInstance->CacheOffset = 0; + HttpInstance->CacheBody = AllocateZeroPool (HttpInstance->CacheLen); + if (HttpInstance->CacheBody == NULL) { + Status = EFI_OUT_OF_RESOURCES; + goto Error2; } + + CopyMem (HttpInstance->CacheBody, Fragment.Bulk + HttpMsg->BodyLength, HttpInstance->CacheLen); + HttpInstance->CacheOffset = 0; + HttpInstance->NextMsg = HttpInstance->CacheBody; } if (Fragment.Bulk != NULL) { FreePool (Fragment.Bulk); Fragment.Bulk = NULL; diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c index 5356cd35c0..94f89f5665 100644 --- a/NetworkPkg/HttpDxe/HttpProto.c +++ b/NetworkPkg/HttpDxe/HttpProto.c @@ -195,10 +195,20 @@ HttpTcpReceiveNotifyDpc ( Length = (UINTN) Wrap->TcpWrap.Rx6Data.FragmentTable[0].FragmentLength; } else { Length = (UINTN) Wrap->TcpWrap.Rx4Data.FragmentTable[0].FragmentLength; } + // + // Record the CallbackData data. + // + HttpInstance->CallbackData.Wrap = (VOID *) Wrap; + HttpInstance->CallbackData.ParseData = Wrap->HttpToken->Message->Body; + HttpInstance->CallbackData.ParseDataLength = Length; + + // + // Parse Body with CallbackData data. + // Status = HttpParseMessageBody ( HttpInstance->MsgParser, Length, Wrap->HttpToken->Message->Body ); diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h index cc6c1eb566..fa57dbfd39 100644 --- a/NetworkPkg/HttpDxe/HttpProto.h +++ b/NetworkPkg/HttpDxe/HttpProto.h @@ -89,10 +89,19 @@ typedef struct { EFI_TLS_CONNECTION_END ConnectionEnd; EFI_TLS_VERIFY VerifyMethod; EFI_TLS_SESSION_STATE SessionState; } TLS_CONFIG_DATA; +// +// Callback data for HTTP_PARSER_CALLBACK() +// +typedef struct { + UINTN ParseDataLength; + VOID *ParseData; + VOID *Wrap; +} HTTP_CALLBACK_DATA; + typedef struct _HTTP_PROTOCOL { UINT32 Signature; EFI_HTTP_PROTOCOL Http; EFI_HANDLE Handle; HTTP_SERVICE *Service; @@ -147,10 +156,11 @@ typedef struct _HTTP_PROTOCOL { // // HTTP message-body parser. // VOID *MsgParser; + HTTP_CALLBACK_DATA CallbackData; EFI_HTTP_VERSION HttpVersion; UINT32 TimeOutMillisec; BOOLEAN LocalAddressIsIPv6; -- 2.17.1.windows.2