File ovmf-bsc1127821-dns-check-packet-size.patch of Package ovmf

Overview Repositories Revisions Requests Users Attributes Meta

File ovmf-bsc1127821-dns-check-packet-size.patch of Package ovmf (Revision cccc10741e6011ba7d49c13ab8b2c77d)

Currently displaying revision cccc10741e6011ba7d49c13ab8b2c77d , Show latest

From e91ad444c474b4ae432fa1a7a494617463271d03 Mon Sep 17 00:00:00 2001
From: Jiaxin Wu <Jiaxin.wu@intel.com>
Date: Mon, 2 Jul 2018 09:20:56 +0800
Subject: [PATCH 1/1] NetworkPkg/DnsDxe: [CVE-2018-12178] Check the received
 packet size before parsing the message.

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 <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wang Fan <fan.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
(cherry picked from commit 84110bbe4bb3a346514b9bb12eadb7586bca7dfd)
---
 NetworkPkg/DnsDxe/DnsImpl.c | 85 +++++++++++++++++++++++++++++++------
 NetworkPkg/DnsDxe/DnsImpl.h |  6 ++-
 2 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index ea3d27da5285..b943ece75708 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1100,8 +1100,9 @@ IsValidDnsResponse (
 
   @param  Instance              The DNS instance
   @param  RxString              Received buffer.
-  @param  Completed             Flag to indicate that Dns response is valid. 
-  
+  @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.
   
@@ -1110,12 +1111,14 @@ 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;
@@ -1141,6 +1144,7 @@ ParseDnsResponse (
   DNS6_RESOURCE_RECORD  *Dns6RR;
 
   EFI_STATUS            Status;
+  UINT32                RemainingLength;
 
   EFI_TPL               OldTpl;
   
@@ -1164,7 +1168,18 @@ ParseDnsResponse (
 
   *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
   //
@@ -1177,23 +1192,39 @@ ParseDnsResponse (
   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;
+
+  //
+  // Check whether the remaining packet length is avaiable or not.
+  //
+  if (RemainingLength <= QueryNameLen + sizeof (DNS_QUERY_SECTION)) {
+    *Completed = FALSE;
+    return EFI_ABORTED;
+  } else {
+    RemainingLength -= (QueryNameLen + sizeof (DNS_QUERY_SECTION));
+  }
+
   //
   // Get query section
   //
-  QuerySection = (DNS_QUERY_SECTION *) (QueryName + AsciiStrLen (QueryName) + 1);
+  QuerySection = (DNS_QUERY_SECTION *) (QueryName + QueryNameLen);
   QuerySection->Type = NTOHS (QuerySection->Type);
   QuerySection->Class = NTOHS (QuerySection->Class);
 
-  //
-  // Get Answer name
-  //
-  AnswerName = (CHAR8 *) QuerySection + sizeof (*QuerySection);
-
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
 
   //
@@ -1327,10 +1358,26 @@ 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.
     //
@@ -1348,6 +1395,17 @@ ParseDnsResponse (
     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.
     //
@@ -1628,6 +1686,7 @@ DnsOnPacketReceived (
   DNS_INSTANCE              *Instance;
 
   UINT8                     *RcvString;
+  UINT32                    Len;
 
   BOOLEAN                   Completed;
   
@@ -1643,17 +1702,15 @@ DnsOnPacketReceived (
 
   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:
 
diff --git a/NetworkPkg/DnsDxe/DnsImpl.h b/NetworkPkg/DnsDxe/DnsImpl.h
index 5fa7f244c257..b64821f485a3 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.h
+++ b/NetworkPkg/DnsDxe/DnsImpl.h
@@ -582,8 +582,9 @@ IsValidDnsResponse (
 
   @param  Instance              The DNS instance
   @param  RxString              Received buffer.
-  @param  Completed             Flag to indicate that Dns response is valid. 
-  
+  @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.
   
@@ -592,6 +593,7 @@ EFI_STATUS
 ParseDnsResponse (
   IN OUT DNS_INSTANCE              *Instance,
   IN     UINT8                     *RxString,
+  IN     UINT32                    Length,
      OUT BOOLEAN                   *Completed
   );
 
-- 
2.20.1

Places

File ovmf-bsc1127821-dns-check-packet-size.patch of Package ovmf (Revision cccc10741e6011ba7d49c13ab8b2c77d)

Places