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.136; helo=mga12.intel.com; envelope-from=jiaxin.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 0A1B3211CBCFE for ; Tue, 26 Feb 2019 00:14:21 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Feb 2019 00:14:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,414,1544515200"; d="scan'208";a="146592196" Received: from jiaxinwu-mobl.ccr.corp.intel.com ([10.239.192.129]) by fmsmga002.fm.intel.com with ESMTP; 26 Feb 2019 00:14:18 -0800 From: Jiaxin Wu To: edk2-devel@lists.01.org Cc: Ye Ting , Fu Siyuan , Wang Fan , Wu Jiaxin Date: Tue, 26 Feb 2019 16:14:16 +0800 Message-Id: <20190226081416.9400-1-Jiaxin.wu@intel.com> X-Mailer: git-send-email 2.17.1.windows.2 Subject: [PATCH v1] NetworkPkg/DnsDxe: Check the received packet size before parsing the message. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Feb 2019 08:14:22 -0000 Fix CVE-2018-12178 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=809 The DNS driver only checks the received packet size against the minimum DNS header size in DnsOnPacketReceived(), later it accesses the QueryName and QuerySection beyond the header scope, which might cause the pointer within DNS driver points to an invalid entry or modifies the memory content beyond the header scope. This patch is to fix above problem. Cc: Ye Ting Cc: Fu Siyuan Cc: Wang Fan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin --- NetworkPkg/DnsDxe/DnsImpl.c | 77 ++++++++++++++++++++++++++++++++----- NetworkPkg/DnsDxe/DnsImpl.h | 2 + 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 89ea755cb2..26a718987c 100644 --- a/NetworkPkg/DnsDxe/DnsImpl.c +++ b/NetworkPkg/DnsDxe/DnsImpl.c @@ -1112,26 +1112,29 @@ IsValidDnsResponse ( /** Parse Dns Response. @param Instance The DNS instance @param RxString Received buffer. + @param Length Received buffer length. @param Completed Flag to indicate that Dns response is valid. @retval EFI_SUCCESS Parse Dns Response successfully. @retval Others Failed to parse Dns Response. **/ EFI_STATUS ParseDnsResponse ( IN OUT DNS_INSTANCE *Instance, IN UINT8 *RxString, + IN UINT32 Length, OUT BOOLEAN *Completed ) { DNS_HEADER *DnsHeader; CHAR8 *QueryName; + UINT32 QueryNameLen; DNS_QUERY_SECTION *QuerySection; CHAR8 *AnswerName; DNS_ANSWER_SECTION *AnswerSection; UINT8 *AnswerData; @@ -1153,10 +1156,11 @@ ParseDnsResponse ( DNS_RESOURCE_RECORD *Dns4RR; DNS6_RESOURCE_RECORD *Dns6RR; EFI_STATUS Status; + UINT32 RemainingLength; EFI_TPL OldTpl; Item = NULL; Dns4TokenEntry = NULL; @@ -1176,10 +1180,21 @@ ParseDnsResponse ( Dns4RR = NULL; Dns6RR = NULL; *Completed = TRUE; Status = EFI_SUCCESS; + RemainingLength = Length; + + // + // Check whether the remaining packet length is avaiable or not. + // + if (RemainingLength <= sizeof (DNS_HEADER)) { + *Completed = FALSE; + return EFI_ABORTED; + } else { + RemainingLength -= sizeof (DNS_HEADER); + } // // Get header // DnsHeader = (DNS_HEADER *) RxString; @@ -1189,26 +1204,42 @@ ParseDnsResponse ( DnsHeader->QuestionsNum = NTOHS (DnsHeader->QuestionsNum); DnsHeader->AnswersNum = NTOHS (DnsHeader->AnswersNum); DnsHeader->AuthorityNum = NTOHS (DnsHeader->AuthorityNum); DnsHeader->AditionalNum = NTOHS (DnsHeader->AditionalNum); + // + // There is always one QuestionsNum in DNS message. The capability to handle more + // than one requires to redesign the message format. Currently, it's not supported. + // + if (DnsHeader->QuestionsNum > 1) { + *Completed = FALSE; + return EFI_UNSUPPORTED; + } + // // Get Query name // QueryName = (CHAR8 *) (RxString + sizeof (*DnsHeader)); + QueryNameLen = (UINT32) AsciiStrLen (QueryName) + 1; + // - // Get query section + // Check whether the remaining packet length is avaiable or not. // - QuerySection = (DNS_QUERY_SECTION *) (QueryName + AsciiStrLen (QueryName) + 1); - QuerySection->Type = NTOHS (QuerySection->Type); - QuerySection->Class = NTOHS (QuerySection->Class); + if (RemainingLength <= QueryNameLen + sizeof (DNS_QUERY_SECTION)) { + *Completed = FALSE; + return EFI_ABORTED; + } else { + RemainingLength -= (QueryNameLen + sizeof (DNS_QUERY_SECTION)); + } // - // Get Answer name + // Get query section // - AnswerName = (CHAR8 *) QuerySection + sizeof (*QuerySection); + QuerySection = (DNS_QUERY_SECTION *) (QueryName + QueryNameLen); + QuerySection->Type = NTOHS (QuerySection->Type); + QuerySection->Class = NTOHS (QuerySection->Class); OldTpl = gBS->RaiseTPL (TPL_CALLBACK); // // Check DnsResponse Validity, if so, also get a valid NET_MAP_ITEM. @@ -1339,14 +1370,30 @@ ParseDnsResponse ( } } Status = EFI_NOT_FOUND; + // + // Get Answer name + // + AnswerName = (CHAR8 *) QuerySection + sizeof (*QuerySection); + // // Processing AnswerSection. // while (AnswerSectionNum < DnsHeader->AnswersNum) { + // + // Check whether the remaining packet length is avaiable or not. + // + if (RemainingLength <= sizeof (UINT16) + sizeof (DNS_ANSWER_SECTION)) { + *Completed = FALSE; + Status = EFI_ABORTED; + goto ON_EXIT; + } else { + RemainingLength -= (sizeof (UINT16) + sizeof (DNS_ANSWER_SECTION)); + } + // // Answer name should be PTR, else EFI_UNSUPPORTED returned. // if ((*(UINT8 *) AnswerName & 0xC0) != 0xC0) { Status = EFI_UNSUPPORTED; @@ -1360,10 +1407,21 @@ ParseDnsResponse ( AnswerSection->Type = NTOHS (AnswerSection->Type); AnswerSection->Class = NTOHS (AnswerSection->Class); AnswerSection->Ttl = NTOHL (AnswerSection->Ttl); AnswerSection->DataLength = NTOHS (AnswerSection->DataLength); + // + // Check whether the remaining packet length is avaiable or not. + // + if (RemainingLength < AnswerSection->DataLength) { + *Completed = FALSE; + Status = EFI_ABORTED; + goto ON_EXIT; + } else { + RemainingLength -= AnswerSection->DataLength; + } + // // Check whether it's the GeneralLookUp querying. // if (Instance->Service->IpVersion == IP_VERSION_4 && Dns4TokenEntry->GeneralLookUp) { Dns4RR = Dns4TokenEntry->Token->RspData.GLookupData->RRList; @@ -1731,10 +1789,11 @@ DnsOnPacketReceived ( ) { DNS_INSTANCE *Instance; UINT8 *RcvString; + UINT32 Len; BOOLEAN Completed; Instance = (DNS_INSTANCE *) Context; NET_CHECK_SIGNATURE (Instance, DNS_INSTANCE_SIGNATURE); @@ -1746,21 +1805,19 @@ DnsOnPacketReceived ( goto ON_EXIT; } ASSERT (Packet != NULL); - if (Packet->TotalSize <= sizeof (DNS_HEADER)) { - goto ON_EXIT; - } + Len = Packet->TotalSize; RcvString = NetbufGetByte (Packet, 0, NULL); ASSERT (RcvString != NULL); // // Parse Dns Response // - ParseDnsResponse (Instance, RcvString, &Completed); + ParseDnsResponse (Instance, RcvString, Len, &Completed); ON_EXIT: if (Packet != NULL) { NetbufFree (Packet); diff --git a/NetworkPkg/DnsDxe/DnsImpl.h b/NetworkPkg/DnsDxe/DnsImpl.h index 90dc054903..45feca2160 100644 --- a/NetworkPkg/DnsDxe/DnsImpl.h +++ b/NetworkPkg/DnsDxe/DnsImpl.h @@ -581,20 +581,22 @@ IsValidDnsResponse ( /** Parse Dns Response. @param Instance The DNS instance @param RxString Received buffer. + @param Length Received buffer length. @param Completed Flag to indicate that Dns response is valid. @retval EFI_SUCCESS Parse Dns Response successfully. @retval Others Failed to parse Dns Response. **/ EFI_STATUS ParseDnsResponse ( IN OUT DNS_INSTANCE *Instance, IN UINT8 *RxString, + IN UINT32 Length, OUT BOOLEAN *Completed ); /** Parse response packet. -- 2.17.1.windows.2